-
-
Notifications
You must be signed in to change notification settings - Fork 4.6k
feat(replay): Add pagination to Playlist view #105701
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
Merged
Merged
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
fb1b6ee
feat(replay): Add pagination to Playlist view
billyvg e978241
hmm make alert sticky at top
billyvg 77eb6d1
embed pagination into replaystable
billyvg 85c7fff
change to use flex + wrap, no expansion
billyvg 8d57061
scroll so selected replay is in middle of table (otherwise, at start …
billyvg File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
Oops, something went wrong.
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.
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.
TBD, but if we want scrolling to work properly in playlist tab, we need to embed pagination into the table, otherwise we can't internally scroll the table. since we have:
| alert |
| table |
| pagination |
ideally, alert should be sticky at the top, along with the table headers, and pagination should only be visible when we scroll to the last row in the table.
In order to have table headers sticky, we would need the scroll to be internal to the table, which means the only way to see pagination only when we reach the last row in the table is for pagination to be embedded in the table. Otherwise, we would need pagination to always be visible as well, which is kind of a waste of pixels.