[web] Delete extraneous code from `PasswordChangeModal`
ClosedPublic
Actions

Authored by tomek on Mar 21 2022, 3:56 AM.

Details

Reviewers

• benschac
atul
• jacek

Commits

rCOMM7b33c27fda1c: [web] Delete extraneous code from `PasswordChangeModal`

Summary

Delete all the code that is not needed in the modal and most of the unused css. The code will be simplified and modified (e.g. change copy) in the next diff. The css will be updated in some later diff, when the design is updated.

password_change_username.png (748×1 px, 80 KB)

Note: the purpose of this diff is to delete components from the modal. Updating its design is done later.

Depends on D3472

Test Plan

Display a modal and check if it contains only relevant components. Also check if error messages are displayed when needed and if password change functionality works.

Diff Detail

Repository

rCOMM Comm

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

tomek created this revision.Mar 21 2022, 3:56 AM

Herald added subscribers: • karol, • adrian, ashoat. · View Herald TranscriptMar 21 2022, 3:56 AM

tomek edited the summary of this revision. (Show Details)Mar 21 2022, 3:57 AM

tomek edited the summary of this revision. (Show Details)

tomek added a child revision: D3474: [web] Directly render code that is no longer conditional.Mar 21 2022, 3:59 AM

Harbormaster completed remote builds in B7501: Diff 10557.Mar 21 2022, 4:03 AM

tomek requested review of this revision.Mar 21 2022, 4:03 AM

tomek edited the test plan for this revision. (Show Details)Mar 21 2022, 4:06 AM

Could you update exactly what you're removing (or planning to remove in this diff)? It's unclear which functionality should stay and which should go. It _seems_ like deleteAccount and logOut are to be removed. But, I want to make sure I'm on the same with you 100%.

• benschac requested changes to this revision.Mar 21 2022, 6:58 AM

This revision now requires changes to proceed.Mar 21 2022, 6:58 AM

In D3473#94465, @benschac wrote:

Could you update exactly what you're removing (or planning to remove in this diff)? It's unclear which functionality should stay and which should go. It _seems_ like deleteAccount and logOut are to be removed. But, I want to make sure I'm on the same with you 100%.

Sure! Everything except password change should be removed: so changing tabs, deleting account and logging out

• benschac accepted this revision.Mar 21 2022, 8:17 AM

This revision is now accepted and ready to land.Mar 21 2022, 8:17 AM

tomek added 1 blocking reviewer(s): atul.Mar 21 2022, 9:20 AM

This revision now requires review to proceed.Mar 21 2022, 9:20 AM

Looks good! Left one question inline (marked "Not Done").

(The other comments were just to keep track of things as reviewing, feel free to disregard)

web/settings/password-change-modal.css
12–14	Searched for `resized-modal-body` in the repo and the only occurrence was in `sidebar-list-modal` which pulls it from a separate `web/style.css` (which doesn't have any styles for `resized-modal-body` anyways)... so this looks fine to remove.
20	Looks like there are no occurrences of `form-pre-footer` in the repo, so this looks fine to remove.
25–35	There aren't any `<textarea>` tags in `password-change-modal`, so these look fine to remove.
54	Couldn't find any unordered lists in `password-change-modal`, so this looks fine to remove.
79	There aren't any occurrences of `.form-subtitle` in the repo (outside of `user-settings-modal.css`, but the style is unused anyway), so this looks fine to remove.
85–114	The `form-text` div was removed from the `password-change-modal` component, so this looks fine to remove.
116–119	No occurrences of `italic` in `password-change-modal` component, so this looks fine to remove.
121–139	There aren't any unordered lists in `password-change-modal`, so this looks fine to remove.
web/settings/password-change-modal.js
36–60	This `Tab` component isn't exported and was only used within `password-change-modal`, so it looks safe to remove. (Looks like the `Tab` component in `user-settings-modal` was pretty much copied from `thread-settings-modal` to begin with...)
63	`currentUserInfo` was used for displaying the `username`, which we aren't doing anymore... so this looks safe to remove.
64	`preRequestUserState` was necessary for `deleteAccount` functionality which is being removed in this diff, so this looks safe to remove.
67–70	The `password-change-modal` doesn't need (and shouldn't have) the ability to delete account, so this is safe to remove.
72	We're removing log out functionality from `password-change-modal`, so this looks safe to remove.
80	No more `Tab` component, so this looks safe to remove.
103–107	We aren't displaying `username` anymore, so this can safely be removed.
109–112	The button that called this function has been removed, so this function can be removed.
114–117	The function that called this is being removed, so this can safely be removed.
125–128	Should we still display the username for the account whose password is being changed? Might not hurt to include? Could imagine this being helpful for users who may have multiple accounts, etc?
155–162	There's no longer account deletion functionality in `password-change-modal`, so this looks safe to remove.
167–174	There's no longer account deletion functionality in `password-change-modal`, so this looks safe to remove.
187–194	There's no longer log out functionality in `password-change-modal`, so this looks safe to remove.
208–223	No more tabs, so this looks safe to remove.
261–263	No more tabs, so this seems safe to remove.
363–405	No more delete account functionality in `password-change-modal`, so this looks safe to remove.