Skip to content
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

Time picker - New states and icons #13851

Merged
merged 8 commits into from
May 31, 2023

Conversation

guidari
Copy link
Contributor

@guidari guidari commented May 22, 2023

Closes #13647

Description

  • Added a new state called warning.
  • Added icons to the current invalid state and to the new warning state.

New

Added a new state called warning.

Testing

  • Go to the Time Picker Playground and test the invalid and warning state to check if they are working as expected.
  • Check if the messages are being displayed correctly

@netlify
Copy link

netlify bot commented May 22, 2023

Deploy Preview for carbon-components-react ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit bc11fc6
🔍 Latest deploy log https://app.netlify.com/sites/carbon-components-react/deploys/647752f6286bb00008f05103
😎 Deploy Preview https://deploy-preview-13851--carbon-components-react.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@netlify
Copy link

netlify bot commented May 22, 2023

Deploy Preview for carbon-elements ready!

Name Link
🔨 Latest commit bc11fc6
🔍 Latest deploy log https://app.netlify.com/sites/carbon-elements/deploys/647752f6ce66240008d35b89
😎 Deploy Preview https://deploy-preview-13851--carbon-elements.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@guidari guidari requested review from a team and thyhmdo and removed request for a team May 22, 2023 12:23
Copy link
Collaborator

@francinelucca francinelucca left a comment

Choose a reason for hiding this comment

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

visuals LGTM! just some implementation questions/suggestions 🔥

packages/react/src/components/TimePicker/TimePicker.tsx Outdated Show resolved Hide resolved
packages/react/src/components/TimePicker/TimePicker.tsx Outdated Show resolved Hide resolved
packages/react/src/components/TimePicker/TimePicker.tsx Outdated Show resolved Hide resolved
@guidari guidari requested a review from francinelucca May 23, 2023 12:21
Copy link
Collaborator

@francinelucca francinelucca left a comment

Choose a reason for hiding this comment

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

🚀

Copy link
Member

@tay1orjones tay1orjones left a comment

Choose a reason for hiding this comment

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

Looks good to me!

Copy link
Member

@tay1orjones tay1orjones left a comment

Choose a reason for hiding this comment

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

Forgot to leave one comment, but it's non-blocking.

@thyhmdo this one's ready for a visual review

Comment on lines +47 to +48
// top/transform used to center invalid icon in IE11
transform: translateY(-50%);
Copy link
Member

Choose a reason for hiding this comment

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

IE11 isn't in our support matrix as of v11

Copy link
Member

@thyhmdo thyhmdo left a comment

Choose a reason for hiding this comment

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

looks good!

@guidari guidari merged commit 589259d into carbon-design-system:main May 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[time picker] update error and warning states
4 participants