Skip to content

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

Open
fshovchko wants to merge 26 commits intoepic/scroll-into-viewfrom
feat/scrollIntoView
Open

feat(esl-utils): ScrollIntoView new implementation#1001
fshovchko wants to merge 26 commits intoepic/scroll-into-viewfrom
feat/scrollIntoView

Conversation

@fshovchko
Copy link
Copy Markdown
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 ala-n and dshovchko 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
Copy Markdown
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

@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
Copy Markdown
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).

@fshovchko fshovchko requested a review from ala-n June 5, 2022 19:47
@ala-n
Copy link
Copy Markdown
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