Tooltip rtl #1830
Tooltip rtl #1830
Conversation
googlebot
added
the
cla: yes
|
@andrewseguin needs rebase |
jelbourn
left a comment
jelbourn
left a comment
There was a problem hiding this comment.
This reminded me that a feature still missing from the tooltip is ensuring that the position changes to be on-screen if it would otherwise go off-screen.
(the connected position strategy handles this already, but updating the transform origin would still be on the tooltip).
src/lib/tooltip/README.md
Outdated
| | --- | --- | --- | | ||
| | `md-tooltip` | `string` | The message to be displayed. | | ||
| | `tooltip-position` | `"above"|"below"|"before"|"after"` | The position of the tooltip. | | ||
| | `tooltip-position` | `"start"|"end"|"top"|"bottom"|"left"|"right"` | The position of the tooltip. | |
There was a problem hiding this comment.
We discussed this in the past and had decided that tooltip would use before and after instead of start and end.
The reasoning was that anything that is positioned relative to some other element would use before and after, and anything where the alignment is global (just based on the page layout) will be start and end.
There was a problem hiding this comment.
Sorry, my brain latched on to before and after and I completely missed that we do want to keep above and below and relatively positioned items as well.
(this is something we need to make sure is consistent before beta)
andrewseguin
force-pushed
the
tooltip-rtl
branch
from
3d32e24 to
6ff7911
Compare
jelbourn
left a comment
jelbourn
left a comment
There was a problem hiding this comment.
LGTM aside minor comments; you can add the merge: ready label after changes
| return { originX: 'center', originY: this.position }; | ||
| } | ||
|
|
||
| const isDirectionLtr = !this._dir || this._dir.value == 'ltr'; |
There was a problem hiding this comment.
optional: might be more readable to just say isRtl
src/lib/tooltip/tooltip.ts
Outdated
| if (this.position == 'right' || | ||
| this.position == 'after' && isDirectionLtr || | ||
| this.position == 'before' && !isDirectionLtr) { | ||
| return { originX: 'end', originY: 'center' }; |
|
@andrewseguin can you rebase / address last comments and then add the |
andrewseguin
force-pushed
the
tooltip-rtl
branch
from
2b69e4a to
82d0f52
Compare
|
Address comments. No longer a breaking change, just adding left and right values. Added merge ready label |
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
4 participants
Closes #1085