Skip to content

Conversation

@geotrev
Copy link
Contributor

@geotrev geotrev commented Apr 29, 2024

Description

Implements a general sibling combinator for Messages in a Field. This way an Input andMessage can have intermediary elements but the latter can still space correctly (like in the case of DatePicker).

Detail

Currently, DatePicker doesn't hvae any spacing when a Message is present. This happens because the menu wrapper sits between them, and the existing adjacent sibling combinator invalidates this scenario:

Screenshot 2024-04-29 at 1 32 05 PM

Compare to Input, which has the correct margin-top when a Message is used:

Screenshot 2024-04-29 at 1 29 22 PM

Now using the general sibling combinator:

Screenshot 2024-04-29 at 1 32 15 PM

Checklist

  • 👌 design updates will be Garden Designer approved (add the designer as a reviewer)
  • 🌐 demo is up-to-date (npm start)
  • ⬅️ renders as expected with reversed (RTL) direction
  • 🤘 renders as expected with Bedrock CSS (?bedrock)
  • 💂‍♂️ includes new unit tests. Maintain existing coverage (always >= 96%)
  • ♿ tested for WCAG 2.1 AA accessibility compliance
  • 📝 tested in Chrome, Firefox, Safari, and Edge

</Datepicker>
</Field>
<Col alignSelf="center">
<div style={{ display: 'flex', justifyContent: 'center' }}>
Copy link
Member

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

<Grid>
<Row justifyContent="center" style={{ height: 'calc(100vh - 80px)' }}>
<Col alignSelf="center">

Copy link
Contributor Author

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.

Copy link
Contributor Author

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.

@geotrev geotrev changed the title 🚧 fix(forms): updates Message to receive margin when intermediaries are present fix(forms): updates Message to receive margin when intermediaries are present Apr 29, 2024
@geotrev geotrev marked this pull request as ready for review April 29, 2024 22:02
Comment on lines 30 to 34
validation: {
options: ['success', 'warning', 'error'],
control: { type: 'radio' },
table: { category: 'Message' }
},
Copy link
Member

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}>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<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

Copy link
Contributor Author

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' }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
table: { category: 'Input' }
table: { category: 'Message' }

[nit] the validationLabel actually belongs to the Message's icon. See https://garden.zendesk.com/components/input#message

Copy link
Contributor Author

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.

@geotrev geotrev merged commit 9b59a96 into main May 1, 2024
@geotrev geotrev deleted the george/dp-input-message branch May 1, 2024 15:07
ze-flo pushed a commit that referenced this pull request May 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

4 participants