[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

Lint Not Applicable

Unit

Tests Not Applicable

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 ↗	(On Diff #10557)	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 ↗	(On Diff #10557)	Looks like there are no occurrences of `form-pre-footer` in the repo, so this looks fine to remove.
25–35 ↗	(On Diff #10557)	There aren't any `<textarea>` tags in `password-change-modal`, so these look fine to remove.
54 ↗	(On Diff #10557)	Couldn't find any unordered lists in `password-change-modal`, so this looks fine to remove.
79 ↗	(On Diff #10557)	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 ↗	(On Diff #10557)	The `form-text` div was removed from the `password-change-modal` component, so this looks fine to remove.
116–119 ↗	(On Diff #10557)	No occurrences of `italic` in `password-change-modal` component, so this looks fine to remove.
121–139 ↗	(On Diff #10557)	There aren't any unordered lists in `password-change-modal`, so this looks fine to remove.
web/settings/password-change-modal.js
36–60 ↗	(On Diff #10557)	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 ↗	(On Diff #10557)	`currentUserInfo` was used for displaying the `username`, which we aren't doing anymore... so this looks safe to remove.
64 ↗	(On Diff #10557)	`preRequestUserState` was necessary for `deleteAccount` functionality which is being removed in this diff, so this looks safe to remove.
67–70 ↗	(On Diff #10557)	The `password-change-modal` doesn't need (and shouldn't have) the ability to delete account, so this is safe to remove.
72 ↗	(On Diff #10557)	We're removing log out functionality from `password-change-modal`, so this looks safe to remove.
80 ↗	(On Diff #10557)	No more `Tab` component, so this looks safe to remove.
103–107 ↗	(On Diff #10557)	We aren't displaying `username` anymore, so this can safely be removed.
109–112 ↗	(On Diff #10557)	The button that called this function has been removed, so this function can be removed.
114–117 ↗	(On Diff #10557)	The function that called this is being removed, so this can safely be removed.
125–128 ↗	(On Diff #10557)	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 ↗	(On Diff #10557)	There's no longer account deletion functionality in `password-change-modal`, so this looks safe to remove.
167–174 ↗	(On Diff #10557)	There's no longer account deletion functionality in `password-change-modal`, so this looks safe to remove.
187–194 ↗	(On Diff #10557)	There's no longer log out functionality in `password-change-modal`, so this looks safe to remove.
208–223 ↗	(On Diff #10557)	No more tabs, so this looks safe to remove.
261–263 ↗	(On Diff #10557)	No more tabs, so this seems safe to remove.
363–405 ↗	(On Diff #10557)	No more delete account functionality in `password-change-modal`, so this looks safe to remove.