-
Notifications
You must be signed in to change notification settings - Fork 3
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
Make preservation task cards expandable #1107
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1107 +/- ##
==========================================
- Coverage 54.72% 54.71% -0.02%
==========================================
Files 105 105
Lines 7696 7696
==========================================
- Hits 4212 4211 -1
- Misses 3226 3228 +2
+ Partials 258 257 -1 ☔ View full report in Codecov by Sentry. |
806621b
to
d8b4a7a
Compare
This commit hides the second and subsequent lines of a multi-line note (i.e. a note with multiple line break character) by default. When the expand icon is clicked the additional lines of the note are displayed. To test this feature you will need to have a task with a multi-line note. I manually inserted a multi-line note into the database to test locally. Here's how it looks in action: |
Fixes #1077. - Show only the first line of a multi-line task note by default - Show an icon on task cards with multi-line notes to toggle display of the subsequent lines of the note - Add css animations for the expansion and contraction of the additional note content
d8b4a7a
to
9953385
Compare
Hi @djjuhasz, Looking good! Though I do have a few feedback thoughts to share, but given that we don't actually have many tasks with long or multi-line text at the moment, it could always wait for a future iteration if we prefer. Anyway, some thoughts:
Thanks! Thoughts and alternatives welcome :) |
813cf70
to
2fabc4e
Compare
@fiver-watson (cc @jraddaoui @mcantelon) I've implemented your feedback by making the entire preservation task card clickable to expand the card and show additional lines of the task note. Unfortunately making the preservation task card clickable has some problems that I'm struggling to resolve. ProblemsI first attempted to make the preservation tasks an accordion component (like I've done with the preservation actions) with each card being an expandable accordion section. The accordion component doesn't work as well for preservation tasks though because many of the tasks won't have additional text, so the accordion section "body" will be empty. Another issue is layout — the accordion section header and body are two different "boxes" (divs) so there will be a visible line between the first line of the note (header), and the subsequent lines (body). My second attempt was to make each preservation task card an HTML button. Clicking the button it expands the preservation task to show the additional text. The problem is that it's awkward mixing tasks that are buttons with ones that are just HTML "div" elements (the default). I think the main issue with mixing HTML element types is accessibility — for instance you can't tab to divs (they aren't interactive), but you can tab to buttons. Other issues about using divs vs. buttons are described here: https://theadminbar.com/accessibility-weekly/divs-are-not-buttons/. My third attempt was making every preservation task card a button, whether it had additional text or not. If the task has only one line of output then I disabled the button so it's not clickable. Disabled buttons don't change color on hover so this strategy works visually, but I worry about the accessibility again. Having a bunch of disabled buttons on the page may be confusing if you are using a screen reader. This approach does solve the tabbing problem as you can still tab to the disabled buttons (if we use the BS "disabled" class). Possible solutions (I can think of)
I think using an accordion component is out because it really assumes that every section will have a header and body content unless we always put some sort of additional content into the tasks. Call to actionI'm curious if any of you can think of any better solutions than the three that I've come up with. |
2fabc4e
to
073fb90
Compare
@djjuhasz It does feel rare to have those expandable sections inside expandable sections. I think the accordion border issue could be fixed overwriting that border with classes/css but I don't know how that would work in terms of accessibility, being inside of another accordion. Another option is to use In general, if you ask me, I don't think it's as important to make the whole card clickable as with the preservation actions, I think just the button would be okay and it would make sense to only allow tabbing to those with extra content/button. If you are inside the tasks you are probably looking for something, I think it may be enough to just have an obvious button where is needed, but I'd wait to hear from @fiver-watson about that. |
073fb90
to
c9aaf9d
Compare
c9aaf9d
to
b70170c
Compare
Fixes #1077.