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

feat(esl-utils): ScrollIntoView new implementation #1001

Open
wants to merge 26 commits into
base: epic/scroll-into-view
Choose a base branch
from

Conversation

fshovchko
Copy link
Contributor

@fshovchko fshovchko commented May 25, 2022

In the scope of this PR i added promisified version for scrollIntoView().

Parameters:
element - the element which should become visible
options (Optional)

For boolean value:
If true, the top of the element will be aligned to the top of the visible area of the scrollable ancestor. Corresponds to scrollIntoViewOptions: {block: "start", inline: "nearest"}. This is the default value.
If false, the bottom of the element will be aligned to the bottom of the visible area of the scrollable ancestor. Corresponds to scrollIntoViewOptions: {block: "end", inline: "nearest"}.

For Object with the following properties:
behavior (Optional) - defines the transition animation. One of auto or smooth. Defaults to auto.
block (Optional) - defines vertical alignment. One of start, center, end, or nearest. Defaults to start.
inline (Optional) - defines horizontal alignment. One of start, center, end, or nearest. Defaults to nearest.

@fshovchko fshovchko requested review from dshovchko and ala-n May 25, 2022 08:42
@fshovchko fshovchko self-assigned this May 25, 2022
@fshovchko fshovchko added the feature New feature label May 25, 2022
@fshovchko fshovchko requested review from a team and NastaLeo and removed request for a team May 25, 2022 10:57
Copy link
Collaborator

@ala-n ala-n left a comment

Choose a reason for hiding this comment

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

There is a couple of critical structural notes here.

But, basically, I'd want to hold the final review here with the architectural rework first. The final API doesn't look great to me at this point.
So just let me summarize the tech requironments, I need a little bit more time to rethink the API first, so marking PR as not ready for merge right now

pages/views/examples/scrollIntoView.njk Outdated Show resolved Hide resolved
pages/views/examples/scrollIntoView.njk Outdated Show resolved Hide resolved
src/modules/esl-footnotes/core/esl-note.ts Outdated Show resolved Hide resolved
src/modules/esl-utils/dom/scroll.ts Outdated Show resolved Hide resolved
@fshovchko fshovchko requested a review from ala-n May 30, 2022 17:14
@ala-n ala-n added this to the ⭐ Release 4.0.0 milestone Jun 2, 2022
Copy link
Collaborator

@ala-n ala-n left a comment

Choose a reason for hiding this comment

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

@fshovchko, please see the use case example pushed to the branch
It looks like scrollIntoView currently does not pass successfully for the dynamic content scenario when page scroll is used (the testing wasn't successful on the clients' side for the same reason).

src/modules/esl-utils/misc/scrollIntoView.ts Outdated Show resolved Hide resolved
@fshovchko fshovchko requested a review from ala-n June 5, 2022 19:47
@ala-n
Copy link
Collaborator

ala-n commented Jun 9, 2022

Needs presentational updates from #1022

@fshovchko fshovchko requested a review from dshovchko June 11, 2022 17:02
@ala-n ala-n removed this from the ⭐ Release 4.0.0 milestone Jun 29, 2022
@ala-n ala-n added this to the EPIC: scrollIntoView extended milestone Sep 2, 2022
@dshovchko dshovchko changed the base branch from main-beta to epic/scroll-into-view October 30, 2022 11:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[🚀esl-utils]: replace scrollIntoView() from scroll utils with new implementation
3 participants