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

displayed promise should probably trigger on after-show if widget is initially invisible #2762

Closed
jasongrout opened this issue Jan 28, 2020 · 9 comments
Assignees
Milestone

Comments

@jasongrout
Copy link
Member

jasongrout commented Jan 28, 2020

Right now, the Javascript side of a widget has a displayed promise that triggers on after-attach. However, some libraries actually need to know when they are visible on the page, not just attached (for example, initially sizing things in bqplot needs to measure its size after the browser has laid out things). This comes up if your widget is in a non-active tab in the stock Tab widget, for example, where the after-attach event triggers because you are added to the DOM, but you are not visible until later, when the user clicks on the tab (and then your widget gets a Lumino after-show event).

Should we:
A. change the semantics of the displayed promise to only trigger on after-attach if the widget is visible, and trigger on the first after-show otherwise?
B. Leave displayed to be tied to after-attach and push the logic down to the widget author. If you really need to wait until you have visibility, set up their own promise that waits for an after-show event after the displayed event? Or if you are already setting up Lumino event processing in your custom widget, perhaps you should tie into the Lumino after-attach event instead of the ipywidgets displayed promise?

I think A probably makes sense? Are there widgets that will want to do some processing after attachment even if they are invisible?

@martinRenou
Copy link
Member

I guess this is a bit related to #2605. We might want the displayed promise to be resolved when the DOM element is visible AND the layout has been applied.

@jasongrout
Copy link
Member Author

We might want the displayed promise to be resolved when the DOM element is visible AND the layout has been applied.

Right now, it looks like the layout is applied by the displayed trigger. Perhaps the layout should be applied immediately on creation. Since it is just changing DOM node styling, I don't think it has to be applied after it is attached. It just has to be applied after the children nodes are built for the querying logic to work.

@vidartf
Copy link
Member

vidartf commented Jan 31, 2020

Note that the attach signal means that the widget is attached to the DOM, and not just floating somewhere. I would want to check if this is significant for anything (e.g. any use of selectors).

@jasongrout
Copy link
Member Author

On the other hand, we could just encourage people to process the Lumino after-attach and after-show Lumino events, and ignore the displayed promise.

@jasongrout
Copy link
Member Author

I would want to check if this is significant for anything (e.g. any use of selectors).

Did anything come up where having displayed trigger on after-attachment even when not visible mattered?

@vidartf
Copy link
Member

vidartf commented Feb 18, 2020

Did anything come up where having displayed trigger on after-attachment even when not visible mattered?

I didn't look very deep, but I think this would be a very narrow case. Happy to disregard until someone comes complaining :)

@vidartf
Copy link
Member

vidartf commented Sep 14, 2021

We discussed this on today's dev meeting, and concluded that it might be best to add three new promises:

  • attached: Triggers after first after-attach message like the current displayed promise, which will be deprecated (log a warning once per session + trigger as it does now).
  • layedOut: Triggers after all layout and styling has completed after display: setLayout before attaching to DOM #2605 (comment)
  • shown: Triggers after the first after-show message

This will allow for third-party libraries to update on their own time, but still reducing the ambiguity of the promise names. Further input is of course welcome 😄

@jasongrout
Copy link
Member Author

This will allow for third-party libraries to update on their own time, but still reducing the ambiguity of the promise names. Further input is of course welcome 😄

I like this idea, and how it provides a gradual upgrade path and removes the ambiguity.

Another approach would be to still deprecate the displayed promise, but document how to listen for the after-attach and after-show messages in Lumino, i.e., punt the problem over to Lumino. The nice thing about this approach is that it doesn't just assume that these messages only happen once, i.e., if we ever reuse a widget view, the messages are appropriately sent.

@vidartf
Copy link
Member

vidartf commented Nov 2, 2021

This was mainly closed by #3300 together with the discussion in this issue. Note also the planned changes in #2605.

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

No branches or pull requests

4 participants