Fork ember-url-hash-polyfill and remove 2-second delay for our use case #1
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
jhuckabee
approved these changes
Oct 10, 2022
lizheym
reviewed
Oct 10, 2022
| @@ -117,7 +115,7 @@ async function setupHashSupport(router: EmberRouter) { | |||
|
|
|||
| const CACHE = new WeakMap<ApplicationInstance, MutationObserver>(); | |||
|
|
|||
| async function eventuallyTryScrollingTo(owner: ApplicationInstance, url?: string) { | |||
| function eventuallyTryScrollingTo(owner: ApplicationInstance, url?: string) { | |||
There was a problem hiding this comment.
Does this need to be called eventually try scrolling if there is no longer a delay?
There was a problem hiding this comment.
Probably not - I was trying to minimize changes to the original project, but if people feel strongly about it, I can rename.
runspired
reviewed
Sep 10, 2023
| return resolve(totalTimeWaited); | ||
| } | ||
|
|
||
| schedule('afterRender', requestTimeCheck); |
There was a problem hiding this comment.
this is the actual bug here, it will always mutate DOM.
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.
See https://github.com/emporatitle/ember-url-hash-polyfill#ember-url-hash-polyfill for the rationale for
ember-url-hash-polyfill's existence.For Empora, the lack of support in Ember for page anchors is preventing us from being able to route back to the action items section of the deal page after creating/updating an action item, and as we continue to build functionality in Ember, it'll be expected that this basic browser functionality will be supported.
I've been able to get the original
ember-url-hash-polyfillworking with Reggie, but it has a hardcoded delay while it waits for animation frames to finish, which means it takes 2 seconds to scroll down to the page anchor, at which point the user has probably already scrolled away to wherever they want to go.Because we don't have a ton of animation in Reggie (and are unlikely to add much in the future), this delay can be removed for our use case.
As recently as August 2022, a member of the core Ember team said that they were working on first-class support for this problem (emberjs/rfcs#709 (comment)), so hopefully this fork is temporary. If it proves not to be, I may PR a change to the original
ember-url-hash-polyfillto make this configurable instead.