Repository navigation
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
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.
Fixes #904
Description
This PR implements highlighting the removed users in the Participants list. Highlighting does not look pretty in dark mode currently (low contrast), but it is IMO the semantically correct way to do it with Bootstrap. They won't "most likely" implement color mode adaptive styling until v6, so we can patch the styles with custom CSS if it feels necessary. The light mode looks ok, and this feature is probably not that often required, so maybe we can live with it for now.
The PR also fixes an unreported (?) issue with exporting the list as CSV/copy: The old conversion from tag list to string used
data.replace(/(<[^>]*>)+/g, ','), which replaces only runs of adjacent tags with a comma. The space between</span>and<span>breaks each run, so instead of collapsing them into one separator, each tag was replaced individually and the spaces survived as list items — yielding the , , entries, like"Aalto, ,Intermediate, ,Visitor,".Finally, the PR fixes several efficiency issues in DOM handling/lookup, detailed explanation below.
Background
Before this change, much of the per-row content (action buttons, inline buttons, row highlighting, checkbox state) was not produced by the table's own rendering pipeline. Instead, a handful of separate
drawevent handlers walked over the rendered DOM after every redraw to patch it up. Every pagination click, sort, filter change or search keystroke therefore triggered several full traversals of the visible rows plus repeated DOM queries, scanning the entire participants array even when only a few rows were affected.This change restructures that work so it scales with the page size (rows currently visible, typically 50) rather than the total number of participants, and removes duplicated scans.
What changed
1. Row content is now built by DataTables column renderers instead of post-draw DOM patching
draw.aplusActionshandler). They are now returned as HTML from the columns'renderfunctions, so DataTables creates them exactly once per row as part of normal row creation, and disposes of them automatically when the row is removed.2. Five draw handlers merged into one
table-dangerfor REMOVED/BANNED rows), selection-checkbox syncing, sticky-header offset recalculation and selection-counter updates ran in separatedraw.*handlers, each traversing the rows independently. They are now a singledraw.aplusMainhandler with one traversal per redraw.3. Removed redundant tag-popover initialization
add_colortag_buttons()was called manually on every draw and after every bulk tag operation, even though it internally installs aMutationObserverthat already initializes popovers on newly added tag badges. The redundant calls were removed, and the function's one-time initialization scan was narrowed from the whole document to the table's own subtree (assets/js/tag_popover.js).4. O(1) participant lookups
Mapfromuser_idto participant is built once; it replaces six places that located participants withparticipants.find(...)inside loops. Applying a tag to thousands of selected students, or opening the "remove tag" modal for a large selection, no longer scans the full participant list once per selected user.5. Code deduplication / minor fixes
ensureFiltersRow()helper. As a side effect this also fixes a latent bug where a filters row built before DataTables initialization was silently discarded by DataTables.selectedFilteredUserIds().t()so labels rendered into rows tolerate the i18n catalog loading late (rows are re-rendered once when translations become ready).Complexity summary
.colortagelements in the document), called on every drawHow to test
Testing
Remember to add or update unit tests for new features and changes.
What type of test did you run?
[ADD A DESCRIPTION ABOUT WHAT YOU TESTED MANUALLY]
Did you test the changes in
Think of what is affected by these changes and could become broken
Translation
Programming style
Have you updated the README or other relevant documentation?
Is it Done?
Clean up your git commit history before submitting the pull request!