RocketCommunicationsInc / astro

Astro UXDS is a collection of guidelines, patterns and components for designing space-based user interface applications.
https://astrouxds.com
Other
115 stars 25 forks source link

Popup menu sometimes doesn't close when close-on-select property is set #839

Closed run2xs closed 1 year ago

run2xs commented 2 years ago

@micahjones13 Thanks for adding the close-on-select property to the rux-pop-up. The popup menu will close correctly if my menu item action navigates to a different route. This did not work before, so it's an improvement. However, I have another menu item that just changes the light/dark theme by changing the class on the body element. When I choose this one, the menu item does not close.

markacianfrani commented 2 years ago

do you mind pasting just your markup (or some minimal version of it) so we can get a better idea of your use case?

run2xs commented 2 years ago

Here's an edited version of the HTML:

`

{{navPage.label}} Set Light Theme Set Dark Theme Logout

`

Here's the call that doesn't close the popup automatically:

changeTheme() { this.isDarkTheme = !this.isDarkTheme; const themeColorClass = this.isDarkTheme ? 'dark-theme' : 'light-theme'; document.body.className = themeColorClass; //todo: remove this when Astro fixes the close bug this.ruxPopUp['el'].hide(); }

run2xs commented 2 years ago

Not sure why I'm losing all the formatting above. I'm attaching a file with the same content.

astro.txt

markacianfrani commented 2 years ago

Thanks for raising this! I think this issue is partly due to Angular's change detection and your use of ngIf. We'll need some more time to explore if there any changes we can make to the component so that you don't have to change your implementation. Since the hide method gives you a workaround, we're going to deescalate this a bit but I'm going to keep the issue open. In the meantime, let us know if you keep encountering this problem in other places

nortonprojects commented 1 year ago

@run2xs If you're using ChangeDetectionStrategy.OnPush, you might try using the default change detection to fix this

markacianfrani commented 1 year ago

We definitely took our time on this one. We weren't able to come up with any built in solutions for this component specifically but are investigating some more open alternatives for future releases.