🪟 🐛 🎨 Fix design issues with CatalogDiff modal #18870
Conversation
| const { formatMessage } = useIntl(); | ||
|
|
||
| const text = formatMessage( | ||
| { | ||
| id: `connection.updateSchema.${diffVerb}`, | ||
| }, | ||
| { | ||
| value: diffCount, | ||
| item: formatMessage({ id: `connection.updateSchema.${diffType}` }, { count: diffCount }), | ||
| } |
There was a problem hiding this comment.
Nit change: This was rendered strangely in the inspector. The string was split into 3 parts in the dom. This change will render the string as a whole.
edmundito
force-pushed
the
edmundito/fix-catalogdiff-styles
branch
from
ecc14b7
to
be4ed4d
Compare
| onClose={() => { | ||
| return null; | ||
| }} |
There was a problem hiding this comment.
This is an optional param
| onClose={() => { | |
| return null; | |
| }} |
There was a problem hiding this comment.
Not optional for catalog diff. Will clean it up.
| @@ -17,23 +17,21 @@ | |||
| font-style: normal; | |||
| font-weight: 500; | |||
| font-size: 10px; | |||
| color: colors.$white; | |||
There was a problem hiding this comment.
This is the wrong text color for this background color. It's not fulfilling WCAG AA contrast ratios. The supposed text colors on those background colors were correct before this PR.
If we're unhappy with the different text colors we can change the background colors to some that allow us having the same text color on top of them.
There was a problem hiding this comment.
This was not an arbitrary change, but the current design has not been updated to reflect this. Neither @Upmitt nor I were aware that this change was made for this reason. I have reverted the change.
Add ModalService to CatalogDiff to fix story
* Update how icons work * Rearrange table headings and cells to align correctly * Add more spacing to the Diff Icon badges * Remove div from DiffHeader and simplify rendering * Remove border-spacing from tables * Move table margins to be set by padding on the panel itself
edmundito
force-pushed
the
edmundito/fix-catalogdiff-styles
branch
from
be4ed4d
to
7ca4490
Compare
|
|
Update CatalogDiff stories to include different use cases
edmundito
changed the title
|
@krishnaglick Added the padding and looks better, and I updated the storybook to preview with different combinations |
* Increase size of CatalogDiff modal Add ModalService to CatalogDiff to fix story * Fix issues with catalog diff field section styles * Update how icons work * Rearrange table headings and cells to align correctly * Add more spacing to the Diff Icon badges * Remove div from DiffHeader and simplify rendering * Remove border-spacing from tables * Move table margins to be set by padding on the panel itself * Update spacing on field section to align more closely with design * Update text color in NumberBadge to be consistent. * Update spacing and alignment in stream row to match design * Restore text color in NumberBadge * Cleanup onClose fn in CatalogDiff story * Add padding bottoom to DiffAccordion Update CatalogDiff stories to include different use cases
What
Fixes the catalog diff modal size along with some styling issues with how fields were being displayed.
Before:
After:
How
Recommended reading order
Top to bottom