opensafely-core / airlock

Other
1 stars 0 forks source link

Htmx the file voting #568

Closed rebkwok closed 1 month ago

rebkwok commented 1 month ago

Submits all the votes via htmx to avoid the page jumping around as reviewers click the voting buttons. There are several elements that need to be updated when a vote changes:

File withdrawal is currently still a full page reload - I think this will be less frequently used and less of an annoyance to users.

Demo of approving/rejecting/resetting in independent review phase (decision remains as incomplete, complete review button enabled and action-required alert shown when vote changes from undecided ): Screencast from 22-07-24 12:25:57.webm

Demo of approving/rejecting in consolidating review phase (displayed decision changes with vote changes, release files button enabled only when approved): Screencast from 22-07-24 12:26:39.webm

Also changes the tree scrolling behaviour to always scroll to the top of the window instead of to the content. This might need to change later depending on whether users prefer it, but it's a simple thing to change.

Fixes #252

rebkwok commented 1 month ago

Happy to look at swapping more things in one go. However, I'm reluctant to change it to a whole page reload because of losing the tree status. While that might not be so annoying for smaller requests, it could be quite annoying for requests with lots of files, and if we were to extend it to e.g. withdrawing file on workspaces it will be REALLY annoying . It also means we unnecessarily reload files that could take a while, like the bigger CSV files.

Or the alternative is to just abandon the OOB swaps altogether, keep the current behaviour, and just fix the tree scrolling-to-content issue, which I did in this PR anyway. I don't think we gain anything by doing a full page reload with htmx. I think that would address the actual annoying page-jumps-around issue that this ticket was about. (Note: I also don't think the solution is to make the tree not scroll at all, because when you click on a new file/directory/group in the tree, you do expect to see the top of the content. If it doesn't scroll at all, and you're part way down a file when you click on a group in the tree, you see the content part way down the group, which is not what you want. Scrolling to the top of the window makes things behave more as I expect, and since you have to scroll up to the top of a page to bring the approve etc buttons into view, clicking on them doesn't make it feel like the page is jumping about)

rebkwok commented 1 month ago

Closing in favour of #573, which just does the tree-scrolling fix