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

fix(block, date-picker, list-item-group, panel, pick-list-group, popover, tip, tip-manager)!: Set default internal heading to a div. #5728

Merged
merged 3 commits into from
Nov 16, 2022

Conversation

driskull
Copy link
Member

@driskull driskull commented Nov 9, 2022

Related Issue: #5099

Summary

fix(block, date-picker, list-item-group, panel, pick-list-group, popover, tip, tip-manager)!: Set default internal heading to a div.

  • Set default heading to be a div to accommodate more audiences
  • Applies to block, date-picker, list-item-group, panel, pick-list-group, popover, tip, and tip-manager



Applies to block, date-picker, list-item-group, panel, pick-list-group, popover, tip, and tip-manager
@driskull driskull requested a review from a team as a code owner November 9, 2022 23:53
@github-actions github-actions bot added the bug Bug reports for broken functionality. Issues should include a reproduction of the bug. label Nov 9, 2022
@driskull driskull added the pr ready for visual snapshots Adding this label will run visual snapshot testing. label Nov 10, 2022
@jcfranco
Copy link
Member

Devs will need to update their code to preserve heading levels, so this would be a breaking change, right?

t9nmanifest.txt Outdated
@@ -29,4 +29,4 @@ src\components\shell-panel\assets\shell-panel\t9n
src\components\time-picker\assets\time-picker\t9n
src\components\tip-manager\assets\tip-manager\t9n
src\components\tip\assets\tip\t9n
src\components\value-list\assets\value-list\t9n
Copy link
Member

Choose a reason for hiding this comment

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

Can you revert this unrelated change?

@driskull
Copy link
Member Author

Devs will need to update their code to preserve heading levels, so this would be a breaking change, right?

Yes, I guess that would be a breaking change if they were not setting any value.

@driskull driskull changed the title fix: Set default heading to be a div to accommodate more audiences. fix!: Set default heading to be a div to accommodate more audiences. Nov 10, 2022
@driskull driskull changed the title fix!: Set default heading to be a div to accommodate more audiences. fix(block, date-picker, list-item-group, panel, pick-list-group, popover, tip, tip-manager)!: Set default heading to a div. Nov 10, 2022
@driskull driskull changed the title fix(block, date-picker, list-item-group, panel, pick-list-group, popover, tip, tip-manager)!: Set default heading to a div. fix(block, date-picker, list-item-group, panel, pick-list-group, popover, tip, tip-manager)!: Set default internal heading to a div. Nov 10, 2022
@driskull
Copy link
Member Author

Updated to breaking change.

@driskull driskull removed the pr ready for visual snapshots Adding this label will run visual snapshot testing. label Nov 14, 2022
@driskull driskull added the pr ready for visual snapshots Adding this label will run visual snapshot testing. label Nov 14, 2022
Copy link
Member

@jcfranco jcfranco left a comment

Choose a reason for hiding this comment

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

Awesome!

Looks like we need a test helper for components + heading levels. Can you create a follow-up issue for this? I think we could add something simple that takes a component + a selector and it asserts internally on headingLevel not being set and when set.

@driskull
Copy link
Member Author

Created issue: #5768

@driskull driskull merged commit 38ca639 into master Nov 16, 2022
@driskull driskull deleted the dris0000/heading-level branch November 16, 2022 17:58
benelan added a commit that referenced this pull request Nov 16, 2022
* master: (24 commits)
  1.0.0-next.632
  feat(pick-list, value-list): Add calciteListFilter event, filteredItems prop, filterText prop and filteredData prop. (#5681)
  fix(block, date-picker, list-item-group, panel, pick-list-group, popover, tip, tip-manager)!: Set default internal heading to a div. (#5728)
  refactor(modal): Update modal to use focus-trap module. (#5676)
  1.0.0-next.631
  fix(input-date-picker): restores mouse clicks on date-picker popup (#5760)
  build(deps): bump loader-utils from 1.4.1 to 1.4.2 (#5764)
  ci(product-label): fix version syntax and use os agnostic newline char (#5762)
  1.0.0-next.630
  ci(pr-bot): don't label dependabot PRs (#5759)
  build(deps): bump chromatic from 6.7.1 to 6.11.4 (#5756)
  fix(combobox): Wrap and break text on long items (#5672)
  build(deps): bump @storybook/addon-interactions from 6.5.9 to 6.5.13 (#5753)
  build(deps): bump type-fest from 3.1.0 to 3.2.0 (#5752)
  build(deps): bump @storybook/addon-a11y from 6.5.12 to 6.5.13 (#5754)
  build(deps): bump postcss from 8.4.18 to 8.4.19 (#5755)
  ci: add chromatic (#5733)
  build(docs): generate docs-json for afd usage (#5748)
  1.0.0-next.629
  fix(inline-editable): Add text-ellipsis when not editing (#5679)
  ...
driskull added a commit that referenced this pull request Nov 16, 2022
…up, popover, tip, tip-manager)!: Set default internal heading to a div. (#5728)"

This reverts commit 38ca639.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Bug reports for broken functionality. Issues should include a reproduction of the bug. pr ready for visual snapshots Adding this label will run visual snapshot testing.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants