-
Notifications
You must be signed in to change notification settings - Fork 6.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(overlay): overlay rtl support #1593
Conversation
this._templatePortal = new TemplatePortal(templateRef, viewContainerRef); | ||
} | ||
|
||
get overlayRef(): OverlayRef { | ||
return this._overlayRef; | ||
} | ||
|
||
get dir(): 'ltr' | 'rtl' { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Type is LayoutDirection
from dir.ts
\
Do you need this getter at all, though, instead of just saying this._dir.value
? It defaults to ltr
in Dir
itself.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, didn't know about that type. Handy. I think I still need the getter in case someone isn't using dir
at all (to keep it optional)
@@ -21,6 +21,9 @@ export class OverlayState { | |||
/** The height of the overlay panel. If a number is provided, pixel units are assumed. **/ | |||
height: number | string; | |||
|
|||
/** The direction of the text in the overlay panel. */ | |||
direction: 'ltr' | 'rtl' = 'ltr'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here for LayoutDirection
@@ -59,6 +60,11 @@ export class OverlayRef implements PortalHost { | |||
} | |||
} | |||
|
|||
/** Updates the text direction of the overlay panel. **/ | |||
private updateDirection() { | |||
this._pane.style.direction = this._state.direction; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of setting the css direction
property, it should probably use do
this._pane.setAttribute('dir', this._state.direction);
LGTM |
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. |
Blocked on #1591 (needs rebase). Adds RTL support to overlay ref and position strategy. APIs need discussion.
r: @jelbourn