Fix rendering of the participant list #1208
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
Backbone requires a unique identifier for each model in a collection to be able to properly sort the models, merge their state when updating them... In the case of participants the id is their "userId" or their "sessionId", depending on whether the participant is a user or a guest. Signed-off-by: Daniel Calviño Sánchez <[email protected]>
The participant list view was rendered on every collection event to overcome the problem of participant views not being rendered, but that caused unneeded renderings, for example when the collection was synced but it had not changed. Now that the id of models in the participant collection is properly set the view for each participant is rendered again as expected when the attributes in its model change, so it is no longer needed to render the participant list on every collection event. Signed-off-by: Daniel Calviño Sánchez <[email protected]>
danxuliu
added
3. to review
backport-request
bug
feature: frontend 🖌️
|
For me this does not update the online status anymore when a user joins/leaves 😿 |
Some of the attribute names, as well as the "checkSharingStatus" call, look like leftovers from a copy-paste; now the proper names are used. Signed-off-by: Daniel Calviño Sánchez <[email protected]>
When the collection is sorted after an update (for example, if a user is promoted to a moderator) there is no need to render again each subview of the participant list; Marionette provides "reorderOnSort" for those cases, which simply reorders the already existing DOM nodes instead of rendering the whole list again. Signed-off-by: Daniel Calviño Sánchez <[email protected]>
The endpoints for moderation operations do not fit the default setup for Backbone models, so the operations are performed directly with AJAX requests on the view. Thus, after a successful request rendering again the view has no visible effect, as the model it is based on has not changed yet. Now the model is updated instead, which will trigger the render with the new state; that state will be later refreshed with the server state (although it should be the same) when the model is synced again. Signed-off-by: Daniel Calviño Sánchez <[email protected]>
When a participant is offline the ".participant-offline" CSS class is added to the element of its view. However, when the view is rendered again the element itself is reused and only its contents are replaced by the template. Thus, the CSS class must be explicitly removed from the element when the view is rendered if the user is now online. As the CSS class is expected (by the CSS rules) to be set on the participant element instead of on the link inside it the CSS class was removed from the template. This makes no difference, though, as "isOffline" was always undefined and thus the class was never set on the link. Signed-off-by: Daniel Calviño Sánchez <[email protected]>
danxuliu
force-pushed
the
fix-rendering-of-the-participant-list
branch
from
fc45ecd
to
b431015
Compare
Cheer up, sad cat, it should be fixed now. |
nickvergessen
approved these changes
Sep 25, 2018
Ivansss
approved these changes
Sep 25, 2018
|
Backported to stable14, should I backport it to stable13 too or not (I have no preference in either way)? |
|
Im not sure we release another 13 |
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.
The participant list was rendered again on every collection event, even if nothing had changed; besides being unnecessary it also caused some flickering and it could also cause the participant menu to be suddenly closed while a moderator had it open to perform some action, like promoting another user.
This pull request fixes it so the participant list is rendered again only when something has actually changed. Moreover, instead of rendering the full list only the specific views of the participants that changed are rendered again.