-
Notifications
You must be signed in to change notification settings - Fork 223
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
ANW-425: Rewrite PUI Collection Organization infinite scroll using native APIs #3014
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
brianzelip
force-pushed
the
ANW-425-pui-infinite-scrollbar
branch
3 times, most recently
from
August 23, 2023 16:15
502be81
to
93bcb5d
Compare
brianzelip
force-pushed
the
ANW-425-pui-infinite-scrollbar
branch
2 times, most recently
from
September 1, 2023 13:57
27aef1e
to
4beec5d
Compare
brianzelip
force-pushed
the
ANW-425-pui-infinite-scrollbar
branch
6 times, most recently
from
September 21, 2023 21:50
a78ba37
to
d9b3aab
Compare
brianzelip
force-pushed
the
ANW-425-pui-infinite-scrollbar
branch
from
September 24, 2023 11:12
9c30b67
to
3660b83
Compare
9 tasks
Replace js scroll and sync mechanisms with intersection observer Replace use of largetree.js with static content on page load Stop dynamically expanding table of contents on scroll Highlight current showing record in table of contents
…ically and I want to be able to see what was different between the current state and whatever comes next...
Update waypoint var name, add nav waypoint var, Whitespace cleanup
Start building out the fetch next WP feature
There is a scroll bug on page load with location.hash The sidebar link text needs tweaking for showing identifiers conditionally and truncating overflow-x with ellipses
Fix horizontal scroll bug
This achieves the desired behavior in FF and Chrome with the expected scoll anchoring-related bugs in Safari
brianzelip
force-pushed
the
ANW-425-pui-infinite-scrollbar
branch
from
September 24, 2023 19:12
3660b83
to
654bd4e
Compare
brianzelip
changed the title
ANW-425: WIP Improve PUI collection organization infinite scroll and largetree behaviors
ANW-425: Improve PUI collection organization infinite scroll and largetree behaviors
Sep 25, 2023
brianzelip
changed the title
ANW-425: Improve PUI collection organization infinite scroll and largetree behaviors
ANW-425: Rewrite PUI Collection Organization infinite scroll using native APIs
Sep 25, 2023
This fixes the problem where scrolling fast could miss populating empty waypoints
donaldjosephsmith
approved these changes
Oct 17, 2023
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This appears to work well with my testing, and nothing jumps out at me as being potentially problematic.
It should be noted that the commit message for this merged PR is a mess! It was auto-squashed-and-merged before I could clean it up and better present it. |
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 PR refactors the PUI Collection Organization's mechanism for handling "infinite scrolling", or the just-in-time loading of information related to a (possibly very large) resource.
The bulk of the changes revolves around replacing the JavaScript-based scrolling environment with a native browser-based scrolling environment. At a high level the code changed from "infinite scroll" and "largetree" to "infinite records" and "infinite tree".
The markup and styles were broadly left alone - just tweaks to some html
id
anddata-*
attributes and creation of new styles.The js is what has changed, but it has remained consistent with the general logic of the existing app, producing essentially the same output in the DOM.
The main files of note are the view template and the
InfiniteRecords
andInfiniteTree
class definitions.The two classes represent similar but different approaches to infinite scrolling given different constraints.
Consistency between this PR and existing code
Differences between this PR and existing code
scrollend
event for Safari and ios.New use of web APIs and language features
This PR introduces use of web APIs and language features that are new to the ArchivesSpace codebase:
<template>
- native element for templating out markupasync/await
- leaner asynchronous code that reads synchronouslyscrollend
event - used for preventing scroll jank when empty record waypoints are observed during programmatic scrolling; a polyfill is included for Safari and iOS, etc.The "infinite scroll", "largetree", and related code is still used in at least the Collection Overview view, so it cannot be fully removed from the PUI codebase yet.
Further details about the new "infinite records" and "infinite tree"
InfiniteRecords
WAYPOINT_SIZE
lengthInfiniteTree
WAYPOINT_SIZE
nodesES6 limitations due to Uglifier build dependency
We experienced CI build failures (via 'ci/dockercloud-stage' which only seems to show up in the GitHub list of checks when it fails) due to the uneven adoption of ES6 syntax by the Uglifier.js dependency which is used for compressing JS assets in production.
for await...of
in the web worker (but potentially not an async generator earlier in the web worker)Public class field fails
The class field fails were solved by moving the fields to the constructor function.
Web worker
for await...of
failThis fail caused a full rewrite of an async generator solution with an ES5 pattern. The ES6 generator code is noted below for provenance (and for reference when Uglifier is replaced!).
Tickets this work reflects