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

Improve link handling in an iframe #45

Open
makhnatkin opened this issue Oct 4, 2024 · 0 comments
Open

Improve link handling in an iframe #45

makhnatkin opened this issue Oct 4, 2024 · 0 comments

Comments

@makhnatkin
Copy link
Collaborator

makhnatkin commented Oct 4, 2024

The current version works fine, but has several issues:

  1. addEventListener('hashchange') is added in every iframe on the page, whereas it could be set just once in the EmbeddedIFrameController.
  2. A timeout had to be added because I cannot ensure, within the iframe, that all other iframes in its parent are fully loaded and that the parent’s height has been fully adjusted.
  3. If multiple iframes contain the same anchor, the scroll might happen to an iframe that is not currently visible on the screen. Ideally, scrolling should happen to the first found anchor across all iframes at the EmbeddedIFrameController level.
  4. I also realized that the addAnchorLinkHandlers method within the iframe won’t work correctly if the link points to a different iframe on the page.

Therefore, I suggest discussing a possible future refactor:

  1. The root controller should provide a loading status — for example, right now we can get information about the loading status of all iframes here, but the result of Promise.all is not utilized anywhere:
    EmbeddedContentRootController.ts#L84
  2. Ideally, we should also ensure that the parent container has finalized the height of all its iframes. (I’ve tried various approaches with ResizeObserver, but haven’t found a clean solution yet.)
  3. I would prefer not to overload the EmbeddedContentRootController with link-handling logic. Instead, this should be configurable via an additional argument or method. There’s already a forEach method, so this could be something like executeOnRootController.
  4. It will also be necessary to somehow dispatch clicks on # links from child iframes to handle them in the root controller (because the link may refer to the header in another iframe). I’m leaning towards something similar to this approach.
  5. The handler for clicking on the link and the handler for the url must be the same – scroll to the first element with the specified id (we search in all iframes). Thus, there will be one handler, and three sources of its trigger: when the page loads, there is already a hash in the url, when the hash is changed, and when clicking on a link with a hash

Notes

  1. execute scrollIntoView in the parent controller
  2. set up the exchange of events between the root controller and the child
  3. take into account the isolated mode
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant