Skip to content
This repository was archived by the owner on May 24, 2024. It is now read-only.

[terra-time-input] Remove placeholder #1235

Merged
merged 35 commits into from
Sep 14, 2020

Conversation

pranav300
Copy link
Contributor

@pranav300 pranav300 commented Sep 10, 2020

Summary

This PR
Removes placeholder from the input.
Adds format to be displayed under the time-input.

Edit: Removed themeable variable --terra-time-input-meridiem-button-group-margin-bottom

Closes #1193

Deployment Link

https://terra-framew-remove-pla-jmtcda.herokuapp.com/components/terra-time-input/time-input/time-input

Testing

Additional Details

Time Input:
Screenshot 2020-09-10 at 5 49 16 PM

12-hour
Screenshot 2020-09-10 at 5 49 31 PM
time input:

Thank you for contributing to Terra.
@cerner/terra

@mjhenkes mjhenkes temporarily deployed to terra-framew-remove-pla-jmtcda September 10, 2020 12:35 Inactive
@mjhenkes mjhenkes temporarily deployed to terra-framew-remove-pla-jmtcda September 10, 2020 12:35 Inactive
Copy link
Contributor

@emilyrohrbough emilyrohrbough left a comment

Choose a reason for hiding this comment

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

@@ -825,6 +831,9 @@ class TimeInput extends React.Component {
/>
</ButtonGroup>
)}
<div id={formatDescriptionId} className={cx('format-text')} aria-label={`Time Format: ${format}`}>
Copy link
Contributor

Choose a reason for hiding this comment

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

"Time Format:" should probably be translated.

Copy link
Contributor Author

@pranav300 pranav300 Sep 11, 2020

Choose a reason for hiding this comment

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

Raised jira for translations.

@mjhenkes mjhenkes temporarily deployed to terra-framew-remove-pla-jmtcda September 10, 2020 18:29 Inactive
@neilpfeiffer neilpfeiffer self-assigned this Sep 10, 2020
@mjhenkes mjhenkes temporarily deployed to terra-framew-remove-pla-jmtcda September 14, 2020 03:10 Inactive
@neilpfeiffer
Copy link
Member

Tested: PR #1235
Part 2 of 3 for issue #1193

Reviewed:

  • Removes placeholders
- Add Date Format Mask as description text below TimeInput



Fix with Followup Issues:

  • Enhancement: [terra-time-input] Add TimeInputField subcomponent #1246 - Add TimeInputField subcomponent

  • Enhancement: [add issue #] Enhance keyboard navigation, today/now shortcut, focus styling 

  • Enhancement: [add issue #] Enhance accessibility to announce the output when full date value is selected




Verified by @neilpfeiffer, Ready to Merge

Pass
UX Review

@neilpfeiffer neilpfeiffer added ⭐ UX Reviewed UX Reviewed and approved. and removed UX Review Required labels Sep 14, 2020
@mjhenkes mjhenkes temporarily deployed to terra-framew-remove-pla-jmtcda September 14, 2020 12:11 Inactive
@mjhenkes mjhenkes temporarily deployed to terra-framew-remove-pla-jmtcda September 14, 2020 12:35 Inactive
@mjhenkes mjhenkes temporarily deployed to terra-framew-remove-pla-jmtcda September 14, 2020 13:51 Inactive
@pranav300
Copy link
Contributor Author

Should these tests be updated to test accessibility now as well? https://github.com/cerner/terra-framework/blob/main/packages/terra-time-input/tests/wdio/time-input-twelve-hour-spec.js#L5

Yes, updated the tests here df91b79

@ryanthemanuel ryanthemanuel merged commit 8c905c5 into main Sep 14, 2020
@ryanthemanuel ryanthemanuel deleted the remove-placeholder-from-time-input branch September 14, 2020 14:49
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove placeholders
8 participants