Integration: LG-2930: [CCP] input components #2318
Merged
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
🦋 Changeset detectedLatest commit: d6031b2 The changes in this PR will be included in the next version bump. This PR includes changesets to release 10 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Size Change: +195 B (0%) Total Size: 1.3 MB
ℹ️ View Unchanged
|
stephl3
commented
Apr 19, 2024
Comment on lines
-49
to
+43
| errorMessage, | ||
| successMessage, | ||
| errorMessage = DEFAULT_MESSAGES.error, | ||
| successMessage = DEFAULT_MESSAGES.success, |
There was a problem hiding this comment.
I previously thought we might want to customize a different default errorMessage or successMessage depending on the input component, but upon further reflection, I think it's fine to set this broadly at the form field level. Open to feedback if we'd prefer to define at the wrapping component level (text input, combobox, etc) since I'm also exporting DEFAULT_MESSAGES from the form field package
There was a problem hiding this comment.
Seems the designs all have the same messages. We can update the default messages per-component if we need to as well
Comment on lines
+13
to
+15
| export const hideContainerStyle = css` | ||
| opacity: 0; | ||
| `; |
There was a problem hiding this comment.
this is conditionally added for cases where a popover may render below an input container but not fully cover the feedback message. adding opacity: 0 will effectively erase the feedback when the menu is open
There was a problem hiding this comment.
Hmm interesting detail! Could you add this as an inline comment
Comment on lines
+95
to
98
| /** token exceptions: background-color value was designated prior to token system */ | ||
| [Theme.Dark]: css` | ||
| color: ${palette.gray.light3}; | ||
| color: ${color.dark.text.primary.default}; | ||
| background-color: ${palette.gray.dark4}; |
There was a problem hiding this comment.
at this point in standardizing components, we'll intentionally continue to leave the palette.gray.dark4 even though it's not in the token system until design decides on how to proceed
There was a problem hiding this comment.
Hmmm. Might be premature to tokenize these then. This could potentially change to specific input color tokens
Comment on lines
+2
to
+7
| '@leafygreen-ui/tokens': minor | ||
| --- | ||
|
|
||
| [LG-2930](https://jira.mongodb.org/browse/LG-2930) | ||
|
|
||
| - Fixes `darkModeBackgroundColors` disabled colors |
There was a problem hiding this comment.
made the assumption that this is still a minor change because it's an aesthetic change but please feel free to correct me if this should instead be major
There was a problem hiding this comment.
I'd even say patch. It's just a bug fix, and doesn't affect API or layout
shaneeza
reviewed
Apr 19, 2024
shaneeza
reviewed
Apr 19, 2024
shaneeza
reviewed
Apr 19, 2024
packages/form-field/src/FormFieldFeedback/FormFieldFeedback.tsx
Outdated
Show resolved
Hide resolved
stephl3
force-pushed
the
steph/ccp-inputs
branch
2 times, most recently
from
dd011b8
to
2223167
Compare
| [LG-2930](https://jira.mongodb.org/browse/LG-2930) | ||
|
|
||
| - Adds and exports `FormFieldFeedback` component | ||
| - Adds and exports `DEFAULT_MESSAGES` constant |
There was a problem hiding this comment.
I don't love this name, but can't think of anything better
packages/form-field/src/FormFieldFeedback/FormFieldFeedback.styles.ts
Outdated
Show resolved
Hide resolved
packages/form-field/src/FormFieldFeedback/FormFieldFeedback.tsx
Outdated
Show resolved
Hide resolved
Comment on lines
+59
to
+60
| color: ${color.light.text.primary.default}; | ||
| background: ${color.light.background.primary.default}; |
There was a problem hiding this comment.
maybe not worth rewriting in this PR, but now that we have the colors token, we can consolidate this into a single function:
export const getInputWrapperModeStyles = (theme: Theme) => css`
color: ${color[theme].text.primary.default};
background: ${color[theme].background.primary.default};
...
`There was a problem hiding this comment.
I started applying this pattern where I could. Unfortunately, until we get design to align on how to handle dark mode backgrounds using gray.dark4 which is currently the value of color.dark.background.secondary.default rather than color.dark.background.primary.default, I think it'll be better to retain the Theme object style for these cases
We've got this design ticket to figure out how to handle those background tokens: https://jira.mongodb.org/browse/LG-4226
TheSonOfThomp
approved these changes
Apr 22, 2024
stephl3
force-pushed
the
steph/ccp-inputs
branch
from
2223167
to
e24e9b9
Compare
stephl3
changed the title
stephl3
force-pushed
the
steph/ccp-inputs
branch
from
3b444b2
to
4dff042
Compare
bruugey
approved these changes
Apr 25, 2024
* [CCP] Combobox updates * Combobox changeset * Clean up styling
* Update placeholder text color * Update spacing * Remove error icon from inside MenuButton * Add error icon and default error message below select * StateFeedback with errorMessage and successMessage * Select changeset * Refactor
* [CCP] NumberInput updates * NumberInput changeset * Use typeScales for unit selector button * Remove redundant stories
* [CCP] PasswordInput changes * PasswordInput changeset * Refactor * Add types tests * Add to README
stephl3
force-pushed
the
steph/ccp-inputs
branch
from
f0b7303
to
d6031b2
Compare
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.
✍️ Proposed changes
tokens
darkModeBackgroundColorsdisabled colorslightModeBorderColorsvariable typotypography
form-field
FormFieldFeedbackcomponentDEFAULT_MESSAGESconstantFormFieldandFormFieldInputContainercomponents to use tokens where possibleFormFieldto useFormFieldFeedbackdate-picker
combobox
errorMessagesuccessMessageprop with defaultnumber-input
UnitSelectcomponent are made in Select: LG-4138 | LG-4150: [CCP] Select updates #2313errorMessagevalidstate withsuccessMessageproppassword-input
errorMessageandsuccessMessagepropsselect
errorMessagevalidstate withsuccessMessageprop🎟 Jira ticket: LG-2930
✅ Checklist
For bug fixes, new features & breaking changes
yarn changesetand documented my changesFor new components
🧪 How to test changes
Review storybook and chromatic