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

refactor(color-picker-hex-input)!: removed intl* properties #6063

Merged

Conversation

anveshmekala
Copy link
Contributor

BREAKING CHANGE: removed intl properties.

  • Removed, intlHex property, aria-label of color-picker-hex-input is set to hex by default.
  • Removed ,intlNoColor property.

note: color-picker-hex-input is internal component.

@github-actions github-actions bot added the refactor Issues tied to code that needs to be significantly reworked. label Dec 15, 2022
@anveshmekala anveshmekala added the pr ready for visual snapshots Adding this label will run visual snapshot testing. label Dec 15, 2022
@anveshmekala anveshmekala marked this pull request as ready for review December 16, 2022 00:09
@anveshmekala anveshmekala requested a review from a team as a code owner December 16, 2022 00:09
@anveshmekala anveshmekala added pr ready for visual snapshots Adding this label will run visual snapshot testing. and removed pr ready for visual snapshots Adding this label will run visual snapshot testing. labels Dec 16, 2022
Copy link
Member

@driskull driskull left a comment

Choose a reason for hiding this comment

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

👍

const hexInputValue = this.formatForInternalInput(value);

return (
<div class={CSS.container}>
<calcite-input
class={CSS.input}
label={intlHex}
label={"hex"}
Copy link
Member

Choose a reason for hiding this comment

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

Does this ever need to be translated?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this needs to be translated. one way is to parse it from the color-picker component as messages similar to what we did for date-picker-month-header.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

color-picker translation has intlHex messages so we can use them.

@anveshmekala anveshmekala added pr ready for visual snapshots Adding this label will run visual snapshot testing. and removed pr ready for visual snapshots Adding this label will run visual snapshot testing. labels Dec 16, 2022
@anveshmekala anveshmekala added pr ready for visual snapshots Adding this label will run visual snapshot testing. and removed pr ready for visual snapshots Adding this label will run visual snapshot testing. labels Dec 16, 2022
Copy link
Member

@driskull driskull left a comment

Choose a reason for hiding this comment

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

👍

/**
* Specifies accessible label for `calcite-input`.
*
* @readonly
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this is readOnly since its set by a parent component.

*
* @readonly
*/
@Prop() hexLabel = "Hex";
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we can just leave it as empty string?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

leaving this empty would break a11y test in color-picker-hex-input with label being empty.

Copy link
Member

Choose a reason for hiding this comment

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

Ok. this is fine then.

@anveshmekala anveshmekala added pr ready for visual snapshots Adding this label will run visual snapshot testing. and removed pr ready for visual snapshots Adding this label will run visual snapshot testing. labels Dec 16, 2022
@anveshmekala anveshmekala merged commit f99b46d into master Dec 16, 2022
@anveshmekala anveshmekala deleted the anveshmekala/remove-intl-props-color-picker-hex-input branch December 16, 2022 19:56
@github-actions github-actions bot added this to the 2023 January Priorities milestone Dec 16, 2022
benelan added a commit that referenced this pull request Dec 16, 2022
* master: (36 commits)
  1.0.0-next.696
  feat(tip,tip-manager): add built-in translations (#6074)
  1.0.0-next.695
  fix(icon, graphic, loader): Set aria-hidden on internal svg elements (#6069)
  1.0.0-next.694
  refactor(modal)!: Renamed CSS variable (#6078)
  1.0.0-next.693
  refactor(color-picker-hex-input)!: removed `intl*` properties (#6063)
  1.0.0-next.692
  feat(shell): Add slots for Modal and Alert (#5983)
  1.0.0-next.691
  refactor(input-date-picker)!: Removed deprecated start, startAsDate, end, endAsDate properties (#6076)
  1.0.0-next.690
  fix(combobox): Fix error when typing a custom value (#6071)
  1.0.0-next.689
  fix(rating): 5312 improve user interface (#5948)
  chore: Clean outdated properties (#6073)
  docs: update component READMEs (#6072)
  chore: Update interface references (#6068)
  chore(action): Clean up action appearance references (#6067)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr ready for visual snapshots Adding this label will run visual snapshot testing. refactor Issues tied to code that needs to be significantly reworked.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants