[Publisher][Design Update] Agency Settings: Account Tab Modals (excluding Jurisdictions) (3/5) #1257 #1327
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ng to GlobalStyles.tsx
…tte, fixed some margin and spacing issues in the Settings.styles, AgencySettings.styles, and AccountSettings.styles files
… for email reminder input on AgencySettingsEmailNotifications
…d spans in the AgencySettingsBasicInfo file
…logic to show display component next to the section labels of AgencySettingsDescription and AgencySettingsUrl
…ommon repo and adjusted spacing on severalelements in the AgencySettings sections.
…encySettingEmailNotifications
…r CheckboxSpacingWrapper in the AgencySettings.styles, added capitialization to the values in the Sectors sectionin the AgencySettingsBasicInfo file
…cySettingDescription and AgencySettingsURL pages
…ncyDescription modals to not persist invalid email or URL on the UI
…and refactored modal logic in AccountSettings
mxosman
reviewed
May 16, 2024
There was a problem hiding this comment.
Hi @corypride! I just played around with this and it looks and feels great! I left a few comments and bumped some old ones, but after those are addressed, I think this will be good to go!
| { message: string } | undefined | ||
| >(undefined); | ||
| const onClickClose = () => { | ||
| setIsSettingInEditMode(!isSettingInEditMode); |
mxosman
reviewed
May 17, 2024
There was a problem hiding this comment.
Hi @corypride! Thank you for all the work so far - let me know if you have any questions on the feedback! It might be easier to go through them in the Files changed tab so you can see the previous conversations as well. I'll need to do another pass as there were a lot of recent changes, but didn't want to delay the initial feedback. Please let me know once everything is ready to review again!
A couple of visual notes:
-
The spacing here looks off compared to the Figma - can you take a look?
-
Can we remove the hover effect on these?
publisher/src/components/Settings/AgencySettingsDescription.tsx
Outdated
Show resolved
Hide resolved
|
|
||
| export const Checkbox = styled.input<{ squareCheckboxConfigs?: boolean }>` | ||
| appearance: ${({ squareCheckboxConfigs }) => | ||
| squareCheckboxConfigs ? "auto" : "none"}; |
There was a problem hiding this comment.
I think we can use the CheckboxOptions component for consistency instead of adjusting the styled and using this? But, let me know if there's a reason that wouldn't work or I'm misunderstanding.
There was a problem hiding this comment.
I used checkbox because it was already being used on the AgencySettingsJurisdictions and Supervisions modals and the styling was already pretty much set consistent with the new styling of the figma (minus the size of the checkboxes which are supposed to be 16X16 and I need to change). I just added the CheckboxLabelWrapper to change the text color, and set the appearance to auto instead of none in order to remove the round checkboxes. Your call... change it or keep it?
There was a problem hiding this comment.
Thank you for explaining your thought process - it's not a huge blocker for me, but I would prefer to reuse the shared Checkbox component. It would keep things consistent and make it easier for future developers (yours + my future selves included), if we decide to adjust all checkboxes, to update one spot instead of also finding custom checkbox components throughout the app to update as well.
… moved some of its functionality to a new function called validateUpdateEmailErrorStates on the AccountSettings page
mxosman
reviewed
May 20, 2024
| @@ -134,8 +134,8 @@ export const AgencySettingsBlock = styled.div<{ | |||
| export const AgencySettingsBlockTitle = styled.div<{ | |||
| isEditModeActive?: boolean; | |||
| configured?: boolean; | |||
| agencySettingsConfigs?: boolean; | |||
There was a problem hiding this comment.
It doesn't look like this is being used either - you can delete.
Giving this comment a bump to delete this unused agencySettingsConfigs
mxosman
reviewed
May 20, 2024
mxosman
reviewed
May 21, 2024
There was a problem hiding this comment.
Hi @corypride! Thank you for addressing all the blocking feedback - it's looking great! One small bug and a one-line deletion request - should be super quick and then this will be approved!
publisher/src/components/Settings/AgencySettingsJurisdictions.tsx
Outdated
Show resolved
Hide resolved
|
Once this is approved & merged in, let's sync up and do all the rebasing together and the final merge into main! |
…ncel button from AgencySettingsJurisdictions modal
mxosman
approved these changes
May 21, 2024
corypride
merged commit 5deca19
into
cory/agency-settings-design-udpates-feature
7 checks passed
corypride
added a commit
that referenced
this pull request
May 21, 2024
* [Publisher]Agency Settings: edit profileDropdownMetadata and create GlobalStyles.tsx (#1271) * [Publisher]Agency Settings: edit profileDropdownMetadata to replace 'Your Account,' 'Agency Settings,' and 'Team Management' with the option 'Settings'. Added GlobalStyles.tsx * [Publisher]GlobalStyles.tsx: add license header and 2 component skeletons * [Publisher]GlobalStyles.tsx: removed extra whitespace characters * [Publisher]GlobalStyles.tsx: ran yarn lint --fix * [Publisher]Settings.tsx and SettingsMenu: refactor and removal * Revert "[Publisher]Settings.tsx and SettingsMenu: refactor and removal" This reverts commit 54373c1. * [Publisher]Agency Settings: refactor of Settings.tsx and removal of SettingsMenu.tsx (#1272) * [Publisher]Agency Settings: edit profileDropdownMetadata to replace 'Your Account,' 'Agency Settings,' and 'Team Management' with the option 'Settings'. Added GlobalStyles.tsx * [Publisher]GlobalStyles.tsx: add license header and 2 component skeletons * [Publisher]GlobalStyles.tsx: removed extra whitespace characters * [Publisher]GlobalStyles.tsx: ran yarn lint --fix * [Publisher]Settings.tsx and SettingsMenu: refactor and removal * [Publisher] Agency Settings: Settings page + Account Tab (2/5) #1256 (#1308) * made major changes to the settings page components and all that is connected * [Publisher][Design Update] Agency Settings: Settings page + Account Tab (2/5) * ran yarn lint --fix * manually removed extra space in code on AgencySettings.styles.tsx * removed unused code from AgencySettingsEmailNotifications and AgencySetting.styles * Removed all files that are in the .gitignore * removed TODO notes from AccountSettings.tsx and added suggested styling to GlobalStyles.tsx * removed unused code, replaced custom colors with colors from the palette, fixed some margin and spacing issues in the Settings.styles, AgencySettings.styles, and AccountSettings.styles files * removed custom color and used color from palette and adjusted spacing for email reminder input on AgencySettingsEmailNotifications * created custom components in AgencySettings.Styles to replace divs and spans in the AgencySettingsBasicInfo file * removed div and added styling to custom component in AgencySettings.styles * created a AgencySettingActionRequiredIndicator component and applied logic to show display component next to the section labels of AgencySettingsDescription and AgencySettingsUrl * removed duplicate input component and used an existing one from the common repo and adjusted spacing on severalelements in the AgencySettings sections. * Added flags to Input.tsx and new styles to Input.styled.tsx * Removed duplicate button componenet * used the checkboxOptions component to replace an input checkbox in AgencySettingEmailNotifications * removed unnecessary styling inheritance and made a meaningful name for CheckboxSpacingWrapper in the AgencySettings.styles, added capitialization to the values in the Sectors sectionin the AgencySettingsBasicInfo file * simplified logic on AgencySettingsActionRequiredIndicator in the AgencySettingDescription and AgencySettingsURL pages * changed a variable name * added settigsViewOptions object in prep for using common tabbed bar component * added TabbedBar component to Settings page * removed unused logic on Input.tsx * removed unused logic on Input.tsx * added close-icon-lg.svg to the assets folder * refactored const SettigsViewOptions to use the tabOptions enum in the Settings pg * removed log statement and removed const declaration from tabOptions in Settings page * moved tabOptions enum outside of the Settings function * modified label on settingViewOptions to remove underscore from Team_Members * cleaned up logic statement in AgencySettingsSupervisions * added logic to DescriptionSection in the AgencySettingsEmailNotifications * modified subpopulations modal by left-aligning options * removed const systemsToDisplayInReadMode from AgencySettingSupervisons because we no longer need the filtering that the const provided * renamed the SettingsTitle component -- SettingsTitleContainer and split the font styling between two new components, SettingsTitle and SettingsSubTitle * updated the 'Learn More' link on the Settings page to point to the set-up-metrics page * replaced styling to the NewInput component in Input.styled.tsx * removed unused code in Input component * updated tabOptions enum on Settings page * changed variable name on Settings page * updated initial value of the const currentSettingsView on the Settings page and updated GuideKey to point to AgencySetting * changed logic for the AgencySettingActionRequiredIndicator to depend on purposeAndFunctionsSetting variable * [Publisher][Design Update] Agency Settings: Account Tab Modals (excluding Jurisdictions) (3/5) #1257 (#1327) * made major changes to the settings page components and all that is connected * [Publisher][Design Update] Agency Settings: Settings page + Account Tab (2/5) * ran yarn lint --fix * manually removed extra space in code on AgencySettings.styles.tsx * removed unused code from AgencySettingsEmailNotifications and AgencySetting.styles * Removed all files that are in the .gitignore * removed TODO notes from AccountSettings.tsx and added suggested styling to GlobalStyles.tsx * removed unused code, replaced custom colors with colors from the palette, fixed some margin and spacing issues in the Settings.styles, AgencySettings.styles, and AccountSettings.styles files * removed custom color and used color from palette and adjusted spacing for email reminder input on AgencySettingsEmailNotifications * created custom components in AgencySettings.Styles to replace divs and spans in the AgencySettingsBasicInfo file * removed div and added styling to custom component in AgencySettings.styles * created a AgencySettingActionRequiredIndicator component and applied logic to show display component next to the section labels of AgencySettingsDescription and AgencySettingsUrl * removed duplicate input component and used an existing one from the common repo and adjusted spacing on severalelements in the AgencySettings sections. * Added flags to Input.tsx and new styles to Input.styled.tsx * Removed duplicate button componenet * used the checkboxOptions component to replace an input checkbox in AgencySettingEmailNotifications * removed unnecessary styling inheritance and made a meaningful name for CheckboxSpacingWrapper in the AgencySettings.styles, added capitialization to the values in the Sectors sectionin the AgencySettingsBasicInfo file * simplified logic on AgencySettingsActionRequiredIndicator in the AgencySettingDescription and AgencySettingsURL pages * changed a variable name * added settigsViewOptions object in prep for using common tabbed bar component * added TabbedBar component to Settings page * removed unused logic on Input.tsx * removed unused logic on Input.tsx * added close-icon-lg.svg to the assets folder * refactored const SettigsViewOptions to use the tabOptions enum in the Settings pg * removed log statement and removed const declaration from tabOptions in Settings page * moved tabOptions enum outside of the Settings function * modified label on settingViewOptions to remove underscore from Team_Members * cleaned up logic statement in AgencySettingsSupervisions * added logic to DescriptionSection in the AgencySettingsEmailNotifications * modified subpopulations modal by left-aligning options * removed const systemsToDisplayInReadMode from AgencySettingSupervisons because we no longer need the filtering that the const provided * renamed the SettingsTitle component -- SettingsTitleContainer and split the font styling between two new components, SettingsTitle and SettingsSubTitle * updated the 'Learn More' link on the Settings page to point to the set-up-metrics page * replaced styling to the NewInput component in Input.styled.tsx * removed unused code in Input component * updated tabOptions enum on Settings page * changed variable name on Settings page * updated initial value of the const currentSettingsView on the Settings page and updated GuideKey to point to AgencySetting * changed logic for the AgencySettingActionRequiredIndicator to depend on purposeAndFunctionsSetting variable * added onClickClose function option to the Modal component, added the Modal component to AccountSettings * added email edit modal to the AccountSetting page * updated Modal and Input to allow custom styling options * started adding custom styling to Modal and Input that will apply to the Settings page modals * started adding custom styling to Modal and Input that will apply to the Settings page modals * added custom styling to AccountSettings Modal and the Modal input * updated styling for the modal on AgencySettingsDescription * updated styling on AgencySettingsURL edit modal * added email validation to AccountSettings page * added email url validation and styling to the AccountSettings page * updated the validateAgencyURL logic * removed unused logic in helperUtils * ran linting * added url validation and styling to the AgencySettings page * ran linting * caught bug in validateURL logic that did not meet figma specs for valid or invalid URL * refactored regex on helpUtils validateAgencyURL function * refactored validatAgencyURL function's regex * removed hover styling on button if the logic determines that there should be agencySettingsConfigs * made adjustments to AccountSettings AccountSettingsDescription and URL according to suggestions in PR #1257 * removed unused logic from Modal.tsx. Adjusted AccountSettings and AgencyDescription modals to not persist invalid email or URL on the UI * adjusted AccountSettings, AgencySettingsDescription, and AgencySettingURL modal * removed uneccessary logic from validateEmail function in helperUtils and refactored modal logic in AccountSettings * updated AgencySettingSupervisions and UnsavedChanges modals according to figma docs * ran linting * refactored title and searchbar area of AgencySettingsJurisdictions page * finished refactoring of searchbar for AgencySettingsJurisdictions page * finished refactoring of searchbar for AgencySettingsJurisdictions page * removed opacity setting on Button component * adjusted the logic in the onClickClose function on the AccountSettings page * renamed editModeAndTypeUpdate function to resetEditModeTypeStates and moved some of its functionality to a new function called validateUpdateEmailErrorStates on the AccountSettings page * updated spacing to all agency and account settings modals * updated validation of URL and Email to check validation on change * ran linting * cleaned up logic on AgencySettings pages removed unused code * fixed bug in AgencySettingURL error checking logic and removed the cancel button from AgencySettingsJurisdictions modal
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description of the change
Created/updated modals in the Account Tab of the Settings page.
Created Name & email modals updated the existing Modal component in the common repo to allow for custom styling via using agencySettingsConfigs flag. Added an X close button. Added styling for email validation. Updated styling for Agency Description modal.
ModalUpdates.mp4
Refs# 1257
Closes #1257 #1258
Type of change
Related issues
Checklists
Development
This box MUST be checked by the submitter prior to merging:
These boxes should be checked by the submitter prior to merging:
Code review
These boxes should be checked by reviewers prior to merging: