Skip to content

fix(popover): positioning and dev-example updates (UIM-447) #467

Merged

Conversation

LeonVay
Copy link
Contributor

@LeonVay LeonVay commented May 25, 2020

@fost please take a look.
Dev example updated.
Add ResizeObserver for monitoring of content resize.

@pimenovoleg pimenovoleg self-requested a review May 25, 2020 08:46
@PositiveJS
Copy link
Contributor

Preview docs changes for f9ddd6c at https://positive-js.github.io/mosaic-previews/pr467-f9ddd6c/

@PositiveJS
Copy link
Contributor

Preview docs changes for 9c90f87 at https://positive-js.github.io/mosaic-previews/pr467-9c90f87/

@LeonVay LeonVay changed the title fix(popover): Popover wrong left positioning Popover positioning and dev-example updates (UIM-447) May 25, 2020
@pimenovoleg pimenovoleg changed the title Popover positioning and dev-example updates (UIM-447) fix(popover): positioning and dev-example updates (UIM-447) May 25, 2020
@@ -440,6 +455,10 @@ export class McPopover implements OnInit, OnDestroy {

private manualListeners = new Map<string, EventListenerOrEventListenerObject>();
private readonly destroyed = new Subject<void>();
// @ts-ignore
private readonly resizeObserver = new ResizeObserver(() => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that you should't use ResizeObserver, will better if you use window resize event, also you should use _ngZone.runOutsideAngular

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not see any possibility to watch\observe popover size dynamic changes (content size changed) without monitoring it via ResizeObserver, otherwise I do not get an idea on how we should determine if the window:resize event should be fired on our side manually.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not see any possibility to watch\observe popover size dynamic changes (content size changed) without monitoring it via ResizeObserver.

For most cases, it will be sufficient to use the window resize event.
In other cases, user must trigger window resize event manually.

For example you can see https://github.com/angular/components/blob/e7c7dce13a5a77ca66a92650b79199da697d0734/src/material-experimental/mdc-slider/slider.ts#L262

@LeonVay
Copy link
Contributor Author

LeonVay commented May 26, 2020

Please NOTE: new attribute for directive added mcPopoverPlacementPriority
By defining this array of strings (possible positions) user can set the order of possible placement for popovers fallback positioning strategy.

@PositiveJS
Copy link
Contributor

Preview docs changes for 93a7d35 at https://positive-js.github.io/mosaic-previews/pr467-93a7d35/

@PositiveJS
Copy link
Contributor

Preview docs changes for 22d4ef0 at https://positive-js.github.io/mosaic-previews/pr467-22d4ef0/

@PositiveJS
Copy link
Contributor

Preview docs changes for 23ba8e2 at https://positive-js.github.io/mosaic-previews/pr467-23ba8e2/

@PositiveJS
Copy link
Contributor

Preview docs changes for 17984fb at https://positive-js.github.io/mosaic-previews/pr467-17984fb/

@PositiveJS
Copy link
Contributor

Preview docs changes for e2ae08e at https://positive-js.github.io/mosaic-previews/pr467-e2ae08e/

@PositiveJS
Copy link
Contributor

Preview docs changes for 55c07f2 at https://positive-js.github.io/mosaic-previews/pr467-55c07f2/

@PositiveJS
Copy link
Contributor

Preview docs changes for 6102c44 at https://positive-js.github.io/mosaic-previews/pr467-6102c44/

@PositiveJS
Copy link
Contributor

Preview docs changes for 496df01 at https://positive-js.github.io/mosaic-previews/pr467-496df01/

@PositiveJS
Copy link
Contributor

Preview docs changes for bbba73a at https://positive-js.github.io/mosaic-previews/pr467-bbba73a/

@PositiveJS
Copy link
Contributor

Preview docs changes for 844ee1c at https://positive-js.github.io/mosaic-previews/pr467-844ee1c/

@pimenovoleg pimenovoleg requested a review from mikeozornin May 28, 2020 08:58
@PositiveJS
Copy link
Contributor

Preview docs changes for c25f232 at https://positive-js.github.io/mosaic-previews/pr467-c25f232/

@PositiveJS
Copy link
Contributor

Preview docs changes for de5e993 at https://positive-js.github.io/mosaic-previews/pr467-de5e993/

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 1 Code Smell

53.8% 53.8% Coverage
0.0% 0.0% Duplication

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants