[Publisher] Agency Settings: Settings page + Account Tab (2/5) #1256 #1308
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
|
Hi Cory! I thought I would get to this by end of day - but I will review first thing tomorrow morning. |
mxosman
reviewed
Apr 16, 2024
There was a problem hiding this comment.
Hi @corypride! So cool to see this starting to take shape! I'm still reviewing, but wanted to get the first batch of reviews out while I keep digging in.
mxosman
reviewed
Apr 16, 2024
mxosman
reviewed
Apr 16, 2024
|
Thanks. I’m on it.
…On Tue, Apr 16, 2024 at 10:36 AM Mahmoud O. ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In publisher/src/components/Settings/AccountSettings.styles.tsx
<#1308 (comment)>
:
> gap: 10px;
+ padding-bottom: 24px;
+ padding-top: 24px;
+ border-bottom: 1px solid #ececec;
I would use colors from the palette instead of custom colors
—
Reply to this email directly, view it on GitHub
<#1308 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/A43WLSAGX4GXTLQRT2BUUDLY5VAPTAVCNFSM6AAAAABGH2HZCSVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDAMBUGAYDIMRRGQ>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
mxosman
reviewed
Apr 16, 2024
publisher/src/components/Settings/AgencySettingsEmailNotifications.tsx
Outdated
Show resolved
Hide resolved
4 tasks
…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.
mxosman
reviewed
Apr 23, 2024
There was a problem hiding this comment.
Hi Cory! I'm still re-reviewing, but in the meantime, two big items that stand out to me are:
- There's a 69k line
package-lock.jsonthat's being pushed up in this PR which we should not include because we haven't installed any packages in this PR - and it will cause issues when the app gets built to be deployed. - Can you remove the duplicate
/Buttoncomponents in the/publisherrepo?
mxosman
reviewed
Apr 23, 2024
publisher/src/components/Settings/AgencySettingsEmailNotifications.tsx
Outdated
Show resolved
Hide resolved
…encySettingEmailNotifications
…r CheckboxSpacingWrapper in the AgencySettings.styles, added capitialization to the values in the Sectors sectionin the AgencySettingsBasicInfo file
mxosman
reviewed
Apr 29, 2024
mxosman
reviewed
Apr 29, 2024
mxosman
reviewed
Apr 30, 2024
publisher/src/components/Settings/AgencySettingsSupervisions.tsx
Outdated
Show resolved
Hide resolved
…s because we no longer need the filtering that the const provided
…it the font styling between two new components, SettingsTitle and SettingsSubTitle
…t-up-metrics page
mxosman
reviewed
May 1, 2024
…s page and updated GuideKey to point to AgencySetting
mxosman
reviewed
May 3, 2024
publisher/src/components/Settings/AgencySettingsDescription.tsx
Outdated
Show resolved
Hide resolved
…on purposeAndFunctionsSetting variable
mxosman
approved these changes
May 6, 2024
There was a problem hiding this comment.
Great work on this, @corypride! I know this was a beast and I appreciate all of your work on this and addressing the feedback. This will be a huge (and very much needed) face-lift for the Publisher. Thank you for helping us out with this, good sir!
corypride
merged commit aff8617
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
RecidivizSettingsPgRefactor2.mp4
Type of change
Related issues
Closes #1256
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: