-
Notifications
You must be signed in to change notification settings - Fork 686
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
Remove jquery #4211
Remove jquery #4211
Conversation
Account for changes made to the UI since the work was started. - On journalist index page, make select all/none apply only to visible sources, not those hidden by an active filter. Add test for this. - Handle confirmation for both types of MFA token reset. - Close Tor slider popup with an X, as done elsewhere. - Fix JS handling of "download unread" button. - Fix some duplicate id attributes. - Add flexbox SASS module. - Improve detection of Tor browsers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Confirmed that the test plan passes (except testing on mobile). I am, however, not a JS expert, so someone else might want to look at the code and see if there's any errors. I mean, I did look at it, but it might need another pass.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this looks great! verified manually that js functionality on both interfaces degrades gracefully with security slider set to highest
👍 from me
@creviera want to also review this one before we merge? |
heyyy tests pass for me! I didn't check the Orfox one. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the source interface, JavaScript is used to suggest Tor Browser with the security slider set to safest.
- Check that when visiting a dev instance with a regular desktop browser, you get the purple banner suggesting Tor Browser, that the instructions link still works, and that clicking the X icon to close the warning fades it out.
- (Optional) For bonus points, check Orfox from a phone or emulator, and verify that you're urged to use a desktop browser. Tested on both
Onion Browser
oniOS
and onTor Browser
onAndroid
. - Finally, visit the site with Tor Browser, with the security slider set to standard, to verify that you're encouraged to move it to safest. The instructions link should result in a popup which you can likewise close with the X icon (this is a UX change; previously this popup would be closed by clicking outside of it).
This change also replaces jQuery usage in the journalist interface.
- Log in, and on the source list, confirm that you can select all then select none, with the checkboxes updating accordingly.
- Type part of the first source's codename in the filter box. Click to select all, then clear the filter box and verify that only the first source was selected.
- Click to select all, then use the filter to show just the first source and click to select none. Clear the filter and confirm that only the first source was deselected.
- Find a source with unread messages, and in that source's row, click the button to download the messages. The button should disappear, and the row should now have the "read" styling.
- Click on a source to navigate to their collection page. Verify that the buttons to select all, unread, or none work as expected.
Approved. 👍
Status
Ready for review
Description of Changes
Removes jQuery. This updates the work done by @kaizensoze for #1233 to account for changes made to the UI since.
Fixes #1233.
Testing
In the source interface, JavaScript is used to suggest Tor Browser with the security slider set to safest.
This change also replaces jQuery usage in the journalist interface.
Deployment
This included an AppArmor change to permit Apache to read an icon that wasn't used before. Verify that when visiting the source interface with Tor Browser with the security slider set to standard, that when you click to see instructions, there is a black X icon in the upper right corner of the popup. If you get a broken link, the AppArmor profile may not have been updated correctly.
Checklist
If you made changes to the server application code:
make ci-lint
) and tests (make -C securedrop test
) pass in the development containerIf you made changes to the system configuration:
If you made non-trivial code changes:
If you made changes to documentation:
make docs-lint
) passed locally