Skip to content
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

Issues with "Check popover validity" in popover target attribute activation behavior #8983

Closed
nt1m opened this issue Mar 3, 2023 · 4 comments
Labels
topic: popover The popover attribute and friends

Comments

@nt1m
Copy link
Member

nt1m commented Mar 3, 2023

  1. Check popover validity there does not specify whether to throw exceptions or not (I think here it would make more sense if the answer was no).
  2. If it doesn't throw exceptions, that check is redundant with the one that the "show popover" or "hide popover" algorithms invoke.
@nt1m
Copy link
Member Author

nt1m commented Mar 3, 2023

@nt1m nt1m added the topic: popover The popover attribute and friends label Mar 3, 2023
@josepharhar
Copy link
Contributor

Thanks for catching this. Exceptions should not be thrown, and yeah the checks are redundant so they can be removed entirely.

@annevk
Copy link
Member

annevk commented Mar 4, 2023

I'll add a fix to #8962. More review of that PR welcome btw.

@annevk
Copy link
Member

annevk commented Mar 4, 2023

Wait, I don't think it's redundant in the hide popover case since presumably we don't want to update the popover invoker? I will fix that instead to ensure it doesn't throw exceptions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: popover The popover attribute and friends
Development

No branches or pull requests

3 participants