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.

Sign up for GitHub

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

feat(srcDoc): added scroll to hash #44

Merged
merged 2 commits into from
Oct 4, 2024
Merged

feat(srcDoc): added scroll to hash #44

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d207989
feat(srcDoc): added scroll to hash
makhnatkin Oct 3, 2024
f0cd1d5
feat(srcDoc): updtaed handleHashChange logic
makhnatkin Oct 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion example/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,13 +60,16 @@ After html text
## Long text example

::: html
<a href="#bottom">to bottom</a>
<h2 id="top">Top</h2>
<p>Lorem ipsum dolor sit amet, consectetur adipiscing elit. Quisque lobortis fermentum placerat. Quisque pretium sagittis laoreet. Curabitur eu sagittis tellus. Pellentesque habitant morbi tristique senectus et netus et malesuada fames ac turpis egestas. Donec laoreet eu enim nec pretium. Phasellus diam odio, porttitor sed pellentesque et, iaculis a nibh. Morbi sit amet ipsum in purus sagittis egestas. Quisque ut varius odio, id varius enim. Vestibulum venenatis turpis et ipsum gravida, vel consectetur nisi lobortis. Fusce turpis orci, facilisis condimentum lectus lobortis, aliquet imperdiet tortor. In placerat, eros id viverra efficitur, lectus justo auctor mauris, at vehicula massa diam ac ipsum. In eu tristique nisl. Duis aliquam maximus consectetur. Sed semper hendrerit lectus interdum iaculis.</p>
<p>Nulla facilisi. Vivamus rutrum vel sem et fringilla. Aliquam erat volutpat. Etiam vel placerat mauris, ac aliquam eros. Nunc accumsan elit ut dolor venenatis, porta laoreet metus euismod. Morbi rutrum dignissim neque ac aliquet. Nam facilisis ac massa non egestas. Pellentesque eu consectetur tellus. Maecenas scelerisque cursus lectus non tempor. Etiam non scelerisque quam. Cras a odio sodales, iaculis magna a, bibendum ex. In et arcu auctor urna placerat interdum ut vel ipsum. Class aptent taciti sociosqu ad litora torquent per conubia nostra, per inceptos himenaeos. Quisque sit amet mi lorem. Suspendisse potenti.</p>
<a href="https://yandex.com">Yandex site</a>
<a href="https://yandex.com">target parent: Yandex site</a>
<p>Phasellus vitae posuere erat. Sed blandit nunc eget sapien ultrices fermentum. Aenean vestibulum facilisis elit sed aliquet. Phasellus posuere et leo id commodo. Praesent tincidunt egestas est a fermentum. Quisque maximus eros dolor, at condimentum eros faucibus in. In commodo augue id purus semper, non semper orci interdum. Phasellus luctus augue id ornare bibendum. Praesent dignissim nisi nec nisi imperdiet, vel pretium dui vestibulum. In condimentum magna odio, ac suscipit lectus accumsan non.</p>
<p>Donec imperdiet tortor vitae ipsum gravida euismod. Donec lobortis orci erat, rutrum consequat orci aliquet non. Curabitur non dui eget orci maximus dapibus id vitae orci. Praesent sed venenatis ipsum. Suspendisse bibendum tincidunt arcu, id tempor felis consectetur non. Sed sit amet purus ultrices, tristique enim eu, efficitur mauris. Curabitur sed efficitur ligula, et varius lectus. Morbi libero purus, eleifend in vehicula eu, sagittis eu lacus. Nulla nisi ligula, mollis eu neque ornare, scelerisque rutrum enim. Duis ut volutpat neque. Vivamus gravida, felis a laoreet auctor, leo lacus mattis lacus, non auctor elit tellus ut odio. Phasellus facilisis efficitur dolor, ut fermentum mi mattis in. Suspendisse potenti. Donec tincidunt nunc eu lacus ultricies, a imperdiet est congue. Maecenas non metus non purus lobortis dignissim. Nulla libero dolor, mattis sit amet massa sed, porttitor fringilla tellus.</p>
<a target="_blank" href="https://yandex.com">target blank: Yandex site</a>
<p>Donec imperdiet tortor vitae ipsum gravida euismod. Donec lobortis orci erat, rutrum consequat orci aliquet non. Curabitur non dui eget orci maximus dapibus id vitae orci. Praesent sed venenatis ipsum. Suspendisse bibendum tincidunt arcu, id tempor felis consectetur non. Sed sit amet purus ultrices, tristique enim eu, efficitur mauris. Curabitur sed efficitur ligula, et varius lectus. Morbi libero purus, eleifend in vehicula eu, sagittis eu lacus. Nulla nisi ligula, mollis eu neque ornare, scelerisque rutrum enim. Duis ut volutpat neque. Vivamus gravida, felis a laoreet auctor, leo lacus mattis lacus, non auctor elit tellus ut odio. Phasellus facilisis efficitur dolor, ut fermentum mi mattis in. Suspendisse potenti. Donec tincidunt nunc eu lacus ultricies, a imperdiet est congue. Maecenas non metus non purus lobortis dignissim. Nulla libero dolor, mattis sit amet massa sed, porttitor fringilla tellus.</p>
<h2 id="bottom">Bottom</h2>
<p>Donec imperdiet tortor vitae ipsum gravida euismod. Donec lobortis orci erat, rutrum consequat orci aliquet non. Curabitur non dui eget orci maximus dapibus id vitae orci. Praesent sed venenatis ipsum. Suspendisse bibendum tincidunt arcu, id tempor felis consectetur non. Sed sit amet purus ultrices, tristique enim eu, efficitur mauris. Curabitur sed efficitur ligula, et varius lectus. Morbi libero purus, eleifend in vehicula eu, sagittis eu lacus. Nulla nisi ligula, mollis eu neque ornare, scelerisque rutrum enim. Duis ut volutpat neque. Vivamus gravida, felis a laoreet auctor, leo lacus mattis lacus, non auctor elit tellus ut odio. Phasellus facilisis efficitur dolor, ut fermentum mi mattis in. Suspendisse potenti. Donec tincidunt nunc eu lacus ultricies, a imperdiet est congue. Maecenas non metus non purus lobortis dignissim. Nulla libero dolor, mattis sit amet massa sed, porttitor fringilla tellus.</p>
<p>Donec imperdiet tortor vitae ipsum gravida euismod. Donec lobortis orci erat, rutrum consequat orci aliquet non. Curabitur non dui eget orci maximus dapibus id vitae orci. Praesent sed venenatis ipsum. Suspendisse bibendum tincidunt arcu, id tempor felis consectetur non. Sed sit amet purus ultrices, tristique enim eu, efficitur mauris. Curabitur sed efficitur ligula, et varius lectus. Morbi libero purus, eleifend in vehicula eu, sagittis eu lacus. Nulla nisi ligula, mollis eu neque ornare, scelerisque rutrum enim. Duis ut volutpat neque. Vivamus gravida, felis a laoreet auctor, leo lacus mattis lacus, non auctor elit tellus ut odio. Phasellus facilisis efficitur dolor, ut fermentum mi mattis in. Suspendisse potenti. Donec tincidunt nunc eu lacus ultricies, a imperdiet est congue. Maecenas non metus non purus lobortis dignissim. Nulla libero dolor, mattis sit amet massa sed, porttitor fringilla tellus.</p>
<a href="#top">to top</a>
Expand Down
4 changes: 2 additions & 2 deletions example/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,14 @@ import {promisify} from 'node:util';
htmlPlugin.transform({
bundle: true,
head: `
<base target="_blank" />
<base target="_parent" />
<style>
h2 {
color: gray;
}
</style>
`,
embeddingMode: 'isolated',
embeddingMode: 'srcdoc',
isolatedSandboxHost: 'http://localhost:5005/runtime.html',
}),
],
Expand Down
33 changes: 30 additions & 3 deletions src/runtime/SrcDocIFrameController.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import {DEFAULT_IFRAME_HEIGHT_PADDING} from '../constants';

import {IEmbeddedContentController} from './IEmbeddedContentController';

const PARENT_LOADED_AND_RESIZED_TIMEOUT = 500;

const validateHostElement: (el: HTMLElement) => asserts el is HTMLIFrameElement = (el) => {
if (!(el instanceof HTMLIFrameElement && el.dataset.yfmSandboxMode === 'srcdoc')) {
throw new Error('Host element for `srcdoc` embedding mode was not configured properly');
Expand Down Expand Up @@ -68,15 +70,15 @@ export class SrcDocIFrameController extends Disposable implements IEmbeddedConte

async initialize() {
await this.instantiateController();

await this.setRootClassNames(this.config.classNames);
await this.setRootStyles(this.config.styles);
this.addAnchorLinkHandlers();

this.updateIFrameHeight(
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
this.host.contentWindow!.document.body.getBoundingClientRect().height,
);

this.addAnchorLinkHandlers();
this.handleHashChange();
}

// finds all relative links (href^="#") and changes their click behavior
Expand Down Expand Up @@ -104,6 +106,31 @@ export class SrcDocIFrameController extends Disposable implements IEmbeddedConte
return this.executeOnController((controller) => controller.setStyles(styles));
}

private scrollToHash() {
const hash = window.location.hash.substring(1);

if (hash) {
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
const document = this.host.contentWindow!.document;
const element = document.getElementById(hash);
if (element) {
element.scrollIntoView({behavior: 'smooth'});
}
}
}

private async handleHashChange() {
// цait until all iframes managed by the parent controller are fully loaded,
// and parent containers have heights properly adjusted.
setTimeout(() => {
this.scrollToHash();
}, PARENT_LOADED_AND_RESIZED_TIMEOUT);

const handleHashChange = () => this.scrollToHash();
window.addEventListener('hashchange', handleHashChange);
this.dispose.add(() => window.removeEventListener('hashchange', handleHashChange));
}
Copy link
Collaborator Author

@makhnatkin makhnatkin Oct 4, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

@brotheroftux @d3m1d0v

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

What do you think of each of the things in the proposals?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

#45


private async instantiateController() {
await ensureIframeLoaded(this.host);

Expand Down
Loading