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

Table of content #199

Merged
merged 53 commits into from
Jul 26, 2022
Merged

Table of content #199

merged 53 commits into from
Jul 26, 2022

Conversation

talyguryn
Copy link
Member

resolves #176

@talyguryn talyguryn marked this pull request as ready for review June 9, 2022 15:09
@neSpecc
Copy link
Member

neSpecc commented Jun 9, 2022

Try to scroll this page: https://docs-stage.codex.so/how-to-connect-to-mongo
Looks like there is a problem with highlighting the current section

@neSpecc
Copy link
Member

neSpecc commented Jun 9, 2022

Let's make the "On this page" heading fixed as well

image

@neSpecc
Copy link
Member

neSpecc commented Jun 9, 2022

Please add some offset above the heading when you scroll to it
image

@neSpecc
Copy link
Member

neSpecc commented Jun 11, 2022

Looks like it highlights the top section. I think, it would be better to highlight the section that is currently at the center of the viewport
image

This reverts commit 18aad62.
@TatianaFomina
Copy link
Contributor

Maybe we should hide table of contents on narrow screens? Seems like it is useless when it is at the bottom of the page

@talyguryn
Copy link
Member Author

it could be used as a quick teleport to headers from the end of the article. actually i don't know do we need to show it on mobile devices. need to ask product owner to make a decision.

Maybe we should hide table of contents on narrow screens? Seems like it is useless when it is at the bottom of the page

src/frontend/js/modules/table-of-content.js Outdated Show resolved Hide resolved
src/frontend/styles/components/table-of-content.pcss Outdated Show resolved Hide resolved
src/frontend/styles/components/table-of-content.pcss Outdated Show resolved Hide resolved
src/frontend/styles/components/table-of-content.pcss Outdated Show resolved Hide resolved
src/frontend/styles/layout.pcss Outdated Show resolved Hide resolved
src/frontend/js/modules/table-of-content.js Outdated Show resolved Hide resolved
src/frontend/js/modules/table-of-content.js Outdated Show resolved Hide resolved
@talyguryn talyguryn requested review from nikmel2803, TatianaFomina, robonetphy and slaveeks and removed request for nikmel2803 July 13, 2022 15:25
* If fixed header is present then calculate scroll position based on it
*/
if (this.fixedHeaderSelector) {
const fixedHeader = document.querySelector(this.fixedHeaderSelector);
Copy link
Member

Choose a reason for hiding this comment

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

don't call dom search on every scroll. Do it once before the hadler.

/**
* Calculate scroll position
*/
let lastKnownScrollPosition = 1 + window.scrollY;
Copy link
Member

Choose a reason for hiding this comment

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

magic number

const fixedHeader = document.querySelector(this.fixedHeaderSelector);

if (fixedHeader) {
lastKnownScrollPosition += fixedHeader.getBoundingClientRect().height;
Copy link
Member

@neSpecc neSpecc Jul 13, 2022

Choose a reason for hiding this comment

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

that will fire reflow on every scroll handler. To prevent this, call this method once before the handler since it wont change with scroll.

src/frontend/js/classes/table-of-content.js Outdated Show resolved Hide resolved
calculateBoundings() {
this.tagsSectionsMap = this.tags.map((tag) => {
const rect = tag.getBoundingClientRect();
const top = rect.top + window.scrollY;
Copy link
Member

Choose a reason for hiding this comment

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

you can add the third parameter like "offset" to resolve the problem like that

image

@talyguryn
Copy link
Member Author

window.requestIdleCallback() no safari support

image

@neSpecc
Copy link
Member

neSpecc commented Jul 14, 2022

window.requestIdleCallback() no safari support

If this method resolves your problem, you can use the polyfill for safari
https://github.com/pladaria/requestidlecallback-polyfill

@talyguryn talyguryn requested a review from neSpecc July 21, 2022 15:44
@talyguryn talyguryn merged commit 213f9d8 into main Jul 26, 2022
@talyguryn talyguryn deleted the feature/table-of-content branch July 26, 2022 15:49
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

Successfully merging this pull request may close these issues.

ui: implement the table of contents
5 participants