Get external link and scroll to element #3506
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
5e-Cleric
added
feature
UI/UX
…existence checks, and update scrollIntoView behavior"
…ed useEffect hooks, and simplified getPageContainingElement function."
|
Now also works with page ids Could use a deployment. |
…n useEffect and scrollToPage function"
Closed
…ent functions, instead getting iframe element by id 'BrewRenderer' inside the functions."
|
How should this be tested? |
Go ahead and open any brew and append the bookmark: You should be able to do it with any brew, with any element or page with id, as they are previously generated. |
|
I'm guessing you will have a lot to say about the scrolling methods, cannot remember why i did it this way. |
5e-Cleric
added
🔍 R4 - Reviewed - Fixed! Awaiting final review⭐
…nto scroll-to-element
|
This should be ready for merge |
dbolack-ab
suggested changes
Oct 19, 2024
dbolack-ab
approved these changes
Oct 19, 2024
`frameDidMount` is equivalent to using iframe.addEventListener('load');
Let's not add a new listener and just use the existing event we already have. Functionality still works.
|
Ok, just cleaned up a bit. Merging now! Thanks @5e-Cleric ! 💯 ⭐ 🎉 |
calculuschild
added
🔍 R5 - Approved for merge 💯
and removed
🔍 R4 - Reviewed - Fixed! Awaiting final review⭐
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This does work, not sure it is perfect.
Instead of having to catch the exact time of load finish of the pages, the function waits 100ms before scrolling.
for this instance, i think this is appropiate, and easier to deal with.
Tested this with 90+ page doc, works fine. Unsetting smooth scroll as it triggers lots of page loads.
Closes #2820 and duplicate #3505
to test, try any brew url +
#id