-
Notifications
You must be signed in to change notification settings - Fork 19
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
Updated to Design System 1.1.0 #504
Conversation
updated less and markup, site looks okay update snapshots
e102b59
to
9ec2101
Compare
src/css/base.less
Outdated
@content_line: var(--gray-40); | ||
// .content__line | ||
@content__line: var(--gray-40); | ||
|
||
// .grid_column__top-divider | ||
@grid_column__top-divider: var(--gray-40); | ||
// .grid__column__top-divider | ||
@grid__column__top-divider: var(--gray-40); | ||
|
||
// .grid_column__top-divider | ||
@grid_column__left-divider: var(--gray-40); | ||
// .grid__column__top-divider | ||
@grid__column__left-divider: var(--gray-40); |
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.
Go ahead and delete any variables that aren't used, if that's any of these.
src/css/App.less
Outdated
.o-mega-menu_group-heading, | ||
.o-mega-menu_content-2-list__featured ul { | ||
.o-mega-menu__group-heading, | ||
.o-mega-menu__content-2-list--featured ul { |
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.
Unrelated to this PR, but we shouldn't be adjusting the mega-menu from within CCDB. This should all be removed and moved to https://github.com/cfpb/consumerfinance.gov/blob/main/cfgov/unprocessed/apps/ccdb-search/css/main.less
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.
Can we just delete then?
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.
Yeah maybe so. This and below.
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.
All mega menu references have been removed
@@ -12,7 +12,7 @@ exports[`component: StackedAreaChart initial state renders without crashing 1`] | |||
className="m-notification_content" |
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.
Do a find and replace for m-notification_content
and update it to m-notification__content
I wonder if you ran the find and replace script again whether it would catch these on a second go 🤔
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.
I tried and it did not find anything
@@ -12,7 +12,7 @@ exports[`component: LineChart initial state renders data lens without crashing 1 | |||
className="m-notification_content" |
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.
Update to m-notification__content
className="input-contains-label_before | ||
input-contains-label_before__search" | ||
className="input-contains-label__before | ||
input-contains-label__before__search" |
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 is a modifier and should be updated to input-contains-label__before--search
STEP_1: '.content_hero', | ||
STEP_2: '.m-pagination_btn-next', | ||
STEP_1: '.content__hero', | ||
STEP_2: '.m-pagination__btn-next', | ||
STEP_3: '.saved_search-panel div:nth-child(2)', |
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.
Do a search for saved_search-panel
and update it to saved__search-panel
package.json
Outdated
"eslint-plugin-jsdoc": "^48.2.3", | ||
"eslint-plugin-react-redux": "^4.1.0", | ||
"glob": "^10.3.12", |
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.
glob
can be removed before this goes in (after the find and replace script is removed)
@@ -29,7 +29,7 @@ describe('initial state', () => { | |||
</Provider>, | |||
); | |||
|
|||
expect(updateLocationHookSpy).toBeCalledTimes(1); | |||
expect(updateLocationHookSpy).toBeCalled(); |
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 hook will correctly be called twice when we update index.js to use strict mode. This test will be rewritten in the future to remove redux-mock-store and use a real redux store.
Additions
Changes
Testing