Enable pagination for main grid view #4958
Merged
Conversation
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 ensures that the grid view in development mode will behave like the grid view in normal mode. Signed-off-by: Daniel Calviño Sánchez <[email protected]>
Signed-off-by: Daniel Calviño Sánchez <[email protected]>
Signed-off-by: Daniel Calviño Sánchez <[email protected]>
This will be needed for proper calculations once pagination is enabled in the full grid view, as the local video will be shown in all pages and thus its slot will not be available. Signed-off-by: Daniel Calviño Sánchez <[email protected]>
Signed-off-by: Daniel Calviño Sánchez <[email protected]>
Besides being cleaner this will make possible to fix the grid jumping back to the first page when the number of videos change. Signed-off-by: Daniel Calviño Sánchez <[email protected]>
Signed-off-by: Daniel Calviño Sánchez <[email protected]>
When the number of participants changed or the window was resized the current page was reset to the first one. However, now that "displayedVideos" is a computed property and no longer reset directly in "makeGrid()" it is no longer necessary to go back to the first page. Moreover, as "numberOfPages" is also a computed property it can be watched to restrict the current page to be always in range. Signed-off-by: Daniel Calviño Sánchez <[email protected]>
When there was no longer an overflow the layout hint event was not triggered with a false value, so even if the window was resized to accomodate the larger number of participants the hint was not automatically removed. Now the hint is removed whenever there is no longer an overflow or the grid is shown as a stripe. Signed-off-by: Daniel Calviño Sánchez <[email protected]>
Instead of being explicitly emitted in "makeGrid" the layout hint is now automatically emitted based on the value of the computed properties that it depends on. Signed-off-by: Daniel Calviño Sánchez <[email protected]>
Signed-off-by: Daniel Calviño Sánchez <[email protected]>
Signed-off-by: Daniel Calviño Sánchez <[email protected]>
Signed-off-by: Daniel Calviño Sánchez <[email protected]>
As it is now possible to paginate in the main grid view the layout hint to change to speaker mode is no longer needed. Signed-off-by: Daniel Calviño Sánchez <[email protected]>
danxuliu
added
2. developing
bug
feature: frontend 🖌️
Signed-off-by: Daniel Calviño Sánchez <[email protected]>
Member
|
Seems to work, but
I would show it the first time it overflows the first page, because when you don't have the mouse moving you won't see that it just started to happen. |
nickvergessen
approved these changes
Jan 15, 2021
Member
Author
|
/backport to stable20.1 |
|
The backport to stable20.1 failed. Please do this backport manually. |
This was referenced Feb 19, 2021
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.
Although the grid component is capable of performing pagination until now it was enabled only in stripe mode. When the call view was in grid mode and the participants did not fit a notification was shown to the user, but the buttons to change between grid view pages were not shown. However, (with some tweaks) the pagination does not require the grid view to be in stripe mode to work, so the buttons are now shown when needed also in the main grid view (centered vertically instead of close to the top, as they would clash with the top bar buttons).
The notification was removed too, as now it does not make sense, at least with its previous message. However I wonder if we should still show the notification with a different message (something like "Too many participants to fit in the window. Please use the buttons in the grid to change between different sets of participants.") when a new page is added so the user realizes it (as just showing the buttons may not be noticeable enough, and they could be hidden if the user has not recently interacted with the grid view).
Note that I tested all this using the
devModeof the grid view. It should be the same when using real participants, but it would be better to test it to ensure that the local participant is still always shown as expected in all pages.Besides that this pull request also fixes the grid view jumping back to the first page when the window is resized or a participant joins or leaves, and it also cleans a bit the code.
Pending: