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
fix(forms): updates Message to receive margin when intermediaries are present #1799
Conversation
</Datepicker> | ||
</Field> | ||
<Col alignSelf="center"> | ||
<div style={{ display: 'flex', justifyContent: 'center' }}> |
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.
Shouldn't be needed. I think you can apply justifyContent
to the Row
instead. Compare
react-components/packages/dropdowns.next/demo/stories/ComboboxStory.tsx
Lines 126 to 128 in e2cd75f
<Grid> | |
<Row justifyContent="center" style={{ height: 'calc(100vh - 80px)' }}> | |
<Col alignSelf="center"> |
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.
Nice 👍🏻 I looked at the Col
props and missed that prop for Row
.
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.
Lookin good I think. Removed the hardcoded width so the input could adjust with the column.
validation: { | ||
options: ['success', 'warning', 'error'], | ||
control: { type: 'radio' }, | ||
table: { category: 'Message' } | ||
}, |
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.
Let's restructure this to behave more like Input or Combobox
- default the message
children
to "Message" - add a Story category section control for toggling the
Message
on - move
validation
to an Input category section and allow it to control validation styling for both the input and the message together (as there is no design recommendation for seeing them happen separately)
<Row style={{ height: 'calc(100vh - 80px)' }}> | ||
<Col textAlign="center" alignSelf="center"> | ||
<Row justifyContent="center" style={{ height: 'calc(100vh - 80px)' }}> | ||
<Col alignSelf="center" xs={12} md={4}> |
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.
<Col alignSelf="center" xs={12} md={4}> | |
<Col textAlign="center" alignSelf="center"> |
...let's avoid the change in order to remain consistent with all other centered non-responsive dropdown type stories
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.
Sure. It's more consistent (compared to what this demo looked like before, which I was attempting to recreate) so I'm all for it.
}, | ||
validationLabel: { | ||
control: { type: 'text' }, | ||
table: { category: 'Input' } |
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.
table: { category: 'Input' } | |
table: { category: 'Message' } |
[nit] the validationLabel
actually belongs to the Message's icon. See https://garden.zendesk.com/components/input#message
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.
Ah, shoot, I see it now in the Input story too. Fixing.
Description
Implements a general sibling combinator for
Message
s in aField
. This way anInput
andMessage
can have intermediary elements but the latter can still space correctly (like in the case ofDatePicker
).Detail
Currently,
DatePicker
doesn't hvae any spacing when aMessage
is present. This happens because the menu wrapper sits between them, and the existing adjacent sibling combinator invalidates this scenario:Compare to
Input
, which has the correctmargin-top
when aMessage
is used:Now using the general sibling combinator:
Checklist
👌 design updates will be Garden Designer approved (add the designer as a reviewer)npm start
)⬅️ renders as expected with reversed (RTL) direction?bedrock
)💂♂️ includes new unit tests. Maintain existing coverage (always >= 96%)♿ tested for WCAG 2.1 AA accessibility compliance