Skip to content

Fixed button to scroll back to original expanded position #6421

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

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

ivioje
Copy link

@ivioje ivioje commented May 2, 2025

Description

This PR fixes #6417

Notes for Reviewers
Fixed button to scroll back to original expanded position

Signed commits

  • Yes, I signed my commits.

Sorry, something went wrong.

ivioje added 2 commits May 2, 2025 02:45
…behavior

Signed-off-by: Rhoda Eboreime <rhodaivioje@gmail.com>
…behavior

Signed-off-by: Rhoda Eboreime <rhodaivioje@gmail.com>
@l5io
Copy link
Contributor

l5io commented May 2, 2025

🚀 Preview for commit 6e0f311 at: https://68142a7d721d8695e56b885c--layer5.netlify.app

@carlosriosilva
Copy link
Contributor

Thanks, @ivioje! @sudhanshutech, will you be reviewing this one?

@Jeffrin2005
Copy link
Contributor

Jeffrin2005 commented May 2, 2025

@ivioje Everything is working good.
Just for a suggestion not related the original issue , if we are expanding more than one card - as we can see in this video , the previously opened thing is collapsed, so for addressing multiple expansion card , can we do anything,

16-fix.mp4

No need to do any commits until the core team member review this PR.

@ivioje
Copy link
Author

ivioje commented May 2, 2025

@ivioje Everything is working good. Just for a suggestion not related the original issue , if we are expanding more than one card - as we can see in this video , the previously opened thing is collapsed, so for addressing multiple expansion card , can we do anything,

16-fix.mp4
No need to do any commits until the code team member review this PR.

Thanks! Yes, something can be done. I can try using getBoundingClientRect() for accurate positioning and also directly reference the current card instead of its container.

@M-DEV-1
Copy link
Member

M-DEV-1 commented May 4, 2025

Screencast.from.2025-05-04.13-10-17.webm

@ivioje, there's an issue while trying to open multiple cards after row 2

@ivioje
Copy link
Author

ivioje commented May 4, 2025

@ivioje Everything is working good. Just for a suggestion not related the original issue , if we are expanding more than one card - as we can see in this video , the previously opened thing is collapsed, so for addressing multiple expansion card , can we do anything,

16-fix.mp4
No need to do any commits until the core team member review this PR.

Screencast.from.2025-05-04.13-10-17.webm
@ivioje, there's an issue while trying to open multiple cards after row 2

@M-DEV-1 yes, that's what @Jeffrin2005 observed too. Do I have a go ahead to fix it?

@M-DEV-1
Copy link
Member

M-DEV-1 commented May 4, 2025

Yes, go for it @ivioje, thanks!!

@M-DEV-1
Copy link
Member

M-DEV-1 commented May 4, 2025

Also, I couldn’t seem to replicate this issue on mobile when I tested, but maybe you could also give it a try on your side to double-check.

@ivioje
Copy link
Author

ivioje commented May 4, 2025

Also, I couldn’t seem to replicate this issue on mobile when I tested, but maybe you could also give it a try on your side to double-check.

It works fine on mobile.

ivioje added 2 commits May 5, 2025 09:22
Signed-off-by: Rhoda Eboreime <rhodaivioje@gmail.com>
@ivioje
Copy link
Author

ivioje commented May 5, 2025

Yes, go for it @ivioje, thanks!!

Done! I've updated the pull request. Thanks!

Copy link

github-actions bot commented May 5, 2025

@l5io
Copy link
Contributor

l5io commented May 5, 2025

🚀 Preview for commit 9e090fb at: https://68187cb23e44f3c7fdb12194--layer5.netlify.app

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
@vishalvivekm
Copy link
Contributor

@ivioje
Thank you for your contribution!
Let's discuss this during the website call today at 7 AM CT.

Add it as an agenda item to the meeting minutes, if you would :)

@vishalvivekm vishalvivekm requested review from M-DEV-1 and vr-varad May 5, 2025 10:21
@l5io
Copy link
Contributor

l5io commented May 5, 2025

🚀 Preview for commit a0b8ab4 at: https://681893ab7d36f745ee489b38--layer5.netlify.app

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
@l5io
Copy link
Contributor

l5io commented May 5, 2025

🚀 Preview for commit 80b4878 at: https://6818a15a7c748655ee862edd--layer5.netlify.app

@ivioje
Copy link
Author

ivioje commented May 13, 2025

Hello @vishalvivekm! I'm just curious, is there a pending task on my end that makes this PR still open?

@leecalcote
Copy link
Member

leecalcote commented May 15, 2025

@Jeffrin2005], perhaps, you can help tie this off....

@Jeffrin2005
Copy link
Contributor

@ivioje lgtm, but a minor issue that i figured out is , when we are expanding the multiple cards but by maintaing the first card without closing it, as you can see, we are going downward to expand the second item,
( though the first expanded card will be collpased )

tes1.mp4

cc : @vishalvivekm @vr-varad

@ivioje
Copy link
Author

ivioje commented May 16, 2025

@ivioje lgtm, but a minor issue that i figured out is , when we are expanding the multiple cards but by maintaing the first card without closing it, as you can see, we are going downward to expand the second item, ( though the first expanded card will be collpased )

tes1.mp4
cc : @vishalvivekm @vr-varad

Hello @Jeffrin2005. I don't see any issue in this. Perhaps you could explain clearly?

The downward movement is so because the previously opened card closes when the current card is opened. As you can see from the screencast you sent, the previous card has a longer height with more content. Therefore, when it closes, the current card scrolls into the view. the downward movement is because of the height of the previous workshop card.

If you expand a card above, it won't have the same behaviour. Also, if you expand a card with a smaller height the downward movement won't be visible.

This works fine to me.

@Jeffrin2005
Copy link
Contributor

@ivioje Oh, got it! My bad : ) Everything else looks good to me.

@vr-varad
Copy link
Contributor

@Jeffrin2005 is this is issue all good to go??

@Jeffrin2005
Copy link
Contributor

@vr-varad yeah. Everything is good.

@vr-varad vr-varad requested a review from vishalvivekm May 29, 2025 15:21
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.

Button doesn't scroll back to original expanded position
8 participants