improve Assignment Progress responsiveness on Safari#2008
improve Assignment Progress responsiveness on Safari#2008barna-isaac wants to merge 2 commits intomainfrom
Conversation
For a group with ~1000 members, sorting by a question (eg: clicking column 3) takes over 30 seconds. After the improvement, it takes about 200ms (just like on Chrome). It seems that Safari really struggles with the `&:has(.class)` css selector, which selectively applies styles to a parent when any (potentially non-direct) children have `.class`. I've moved this decision to the application logic, and assignment progress is now just as performant on Safari as it was in Chrome.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2008 +/- ##
==========================================
+ Coverage 43.19% 43.21% +0.02%
==========================================
Files 575 575
Lines 24746 24748 +2
Branches 8214 8187 -27
==========================================
+ Hits 10690 10696 +6
+ Misses 14007 13376 -631
- Partials 49 676 +627 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
jacbn
left a comment
jacbn
left a comment
There was a problem hiding this comment.
Works nicely, thanks for looking into profiling on Safari. We can simplify the classes a little though!
| {progress && progress.length > 0 ? <> | ||
| <div className={classNames("assignment-progress-table-wrapper border", {"rounded-3": isAda})} ref={scrollContainerRef}> | ||
| <table ref={tableRef} className="progress-table w-100"> | ||
| <table ref={tableRef} className={classNames({['progress-table']: true, ['w-100']: true, ['has-selected']: questions.some((_, index) => index === selectedQuestionIndex)})}> |
There was a problem hiding this comment.
You can just put plain strings inside classNames, it has the same effect as {string: true}:
| <table ref={tableRef} className={classNames({['progress-table']: true, ['w-100']: true, ['has-selected']: questions.some((_, index) => index === selectedQuestionIndex)})}> | |
| <table ref={tableRef} className={classNames('progress-table w-100', {'has-selected': selectedQuestionIndex !== undefined})}> |
(was there a reason we needed to check whether the index was in questions? if not, just a defined-ness check is enough)
There was a problem hiding this comment.
We can reduce the scope of the changes by keeping the .progress-table.has-selected SCSS inside a single parent. It's entirely nitpicky as the output is exactly the same, but good practice to minimise PR size. I've committed this separately as Git split your changes into too many to comment over –– but notice there are only two lines changed total now :)
2 participants
For a group with ~1000 members (for example: https://isaacscience.org/assignment_progress/550781), sorting by a question (eg: clicking column 3) takes over 30 seconds. After the improvement, it takes about 200ms (just like on Chrome). It seems that Safari really struggles with the
&:has(.class)css selector, which selectively applies styles to a parent when any (potentially nested) children have.class.I've moved this decision to the application logic, and assignment progress is now just as performant on Safari as it was in Chrome.