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.

Sign up for GitHub

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

Jump to bottom

feat(action-bar, action-pad, action-group): Add label properties for group context #7415

Merged
merged 19 commits into from
Aug 23, 2023
Merged

feat(action-bar, action-pad, action-group): Add label properties for group context #7415

merged 19 commits into from
Aug 23, 2023

Conversation

driskull
Copy link
Member

@driskull driskull commented Aug 1, 2023 •
edited
Loading

Related Issue: #6904

Summary

  • Adds label prop to calcite-action-group
  • Adds actionsEndGroupLabel prop to calcite-action-bar
  • Adds actionsEndGroupLabel prop to calcite-action-pad
  • Refactor calcite-action-group css for additional element
  • Add test

@driskull driskull changed the title feat(action-group): Add label prop for group context. #6904 feat(action-group): Add label prop for group context Aug 1, 2023
@github-actions github-actions bot added the enhancement Issues tied to a new feature or request. label Aug 1, 2023
@driskull driskull added the pr ready for visual snapshots Adding this label will run visual snapshot testing. label Aug 1, 2023
@driskull driskull 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 Aug 1, 2023
@driskull driskull 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 Aug 1, 2023
@driskull driskull 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 Aug 1, 2023
@driskull driskull 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 Aug 1, 2023
@driskull driskull marked this pull request as ready for review August 1, 2023 22:40
@driskull driskull requested a review from a team as a code owner August 1, 2023 22:40
@driskull
Copy link
Member Author

driskull commented Aug 2, 2023

@geospatialem can you test again?

geospatialem
geospatialem reviewed Aug 2, 2023
View reviewed changes
Copy link
Member

@geospatialem geospatialem left a comment

Choose a reason for hiding this comment

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

In terms of a11y, good to go. 🚀 For final code review prior to merging, defer to @jcfranco.

jcfranco
jcfranco approved these changes Aug 2, 2023
View reviewed changes
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.

🚀👨‍🚀🪐

@driskull
Copy link
Member Author

driskull commented Aug 2, 2023

@geospatialem I moved the issue to the July milestone. Just FYI

@driskull driskull 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 Aug 2, 2023
@driskull driskull added the pr ready for visual snapshots Adding this label will run visual snapshot testing. label Aug 4, 2023
@driskull driskull 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 Aug 4, 2023
@driskull driskull changed the title feat(action-group): Add label prop for group context feat(action-bar, action-pad, action-group): Add label prop for group context Aug 4, 2023
@driskull driskull changed the title feat(action-bar, action-pad, action-group): Add label prop for group context feat(action-bar, action-pad, action-group): Add label properties for group context Aug 4, 2023
driskull added 2 commits August 8, 2023 09:29
@driskull
rename prop
a81e01b
@driskull
Merge branch 'main' into dris0000/action-group-label
849a633 
# Conflicts:
#	packages/calcite-components/src/components/action-bar/action-bar.tsx
#	packages/calcite-components/src/components/action-pad/action-pad.tsx
@github-actions
Copy link
Contributor

This PR has been automatically marked as stale because it has not had recent activity. Please close your PR if it is no longer relevant. Thank you for your contributions.

@github-actions github-actions bot added the Stale Issues or pull requests that have not had recent activity. label Aug 16, 2023
@driskull driskull removed the Stale Issues or pull requests that have not had recent activity. label Aug 16, 2023
@driskull
Merge branch 'main' into dris0000/action-group-label
a859f1e 
# Conflicts:
#	packages/calcite-components/src/components/action-bar/action-bar.tsx
#	packages/calcite-components/src/components/action-pad/action-pad.tsx
jcfranco
jcfranco requested changes Aug 23, 2023
View reviewed changes
packages/calcite-components/src/components/action-pad/action-pad.tsx
/**
* Specifies the accessible label for the last action-group.
*/
@Prop() actionsEndGroupLabel: string;
Copy link
Member

Choose a reason for hiding this comment

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

Can this be renamed to be closer to what this actions group represents? The current name may be too tied to the implementation since there's no other hint API-wise about this being an action-group, right? LMK if I'm missing something.

This applies to action-bar too.

Copy link
Member Author

Choose a reason for hiding this comment

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

@geospatialem @jcfranco can you help with the name? I'm not sure what the group represents, its the action group at the end which may contain the expand/collapse button along with whatever else they slot there.

Copy link
Member

Choose a reason for hiding this comment

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

Is there another way we commonly refer to that group or section? It houses the expand/collapse button and its associated tooltip, right?

Copy link
Member Author

Choose a reason for hiding this comment

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

correct. I'm not sure how else we refer to it. Maybe design can elaborate on why its separated and its intention.

Copy link
Member

Choose a reason for hiding this comment

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

For consistency across the design system WDYT about using label as the prop name, and based on design's expertise describe the intent?

Copy link
Member Author

Choose a reason for hiding this comment

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

@jcfranco what do you think? My only hesitation is that what if we need a label for the actual action-pad or action-bar in the future?

Copy link
Member

Choose a reason for hiding this comment

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

I share the same concern. So far, I think we've been using label for the default/main part of a component. This one seems more specialized.

Copy link
Member Author

Choose a reason for hiding this comment

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

Would groupLabel be acceptable? @jcfranco @geospatialem

Copy link
Member

@jcfranco jcfranco Aug 23, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Apologies, I missed the fact that we allow forwarding slotted content to the actions group, so it's not exclusively tied to expand/collapse + tooltip.

The current name (actionsEndGroupLabel) is fitting. Sorry about the hassle! 😅

@driskull
Revert "review fixes"
9377544 
This reverts commit ca62990.
@driskull driskull 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 Aug 23, 2023
jcfranco
jcfranco approved these changes Aug 23, 2023
View reviewed changes
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.

🚀

@driskull driskull merged commit b34f36d into main Aug 23, 2023
@driskull driskull deleted the dris0000/action-group-label branch August 23, 2023 20:17
@github-actions github-actions bot added this to the 2023 August Priorities milestone Aug 23, 2023
@calcite-admin calcite-admin mentioned this pull request Aug 23, 2023
Merged
benelan added a commit that referenced this pull request Aug 24, 2023
@benelan
Merge remote-tracking branch 'origin/main' into benelan/6706-integer-…
2b3738c 
…input

* origin/main: (29 commits)
  chore: release next
  refactor(combobox-item): drop unused variable (#7587)
  fix(combobox): update combobox height to follow design spec (#7558)
  fix(split-button): update divider and borders to follow design spec (#7568)
  fix(segmented-control): refresh items when added dynamically (#7567)
  chore: release next
  feat(action-bar, action-pad, action-group): Add label properties for group context (#7415)
  test(modal, shell): add delay to recent unstable stories (#7589)
  chore: release next
  fix(modal): handle removal of open attribute and prevent multiple beforeClose calls (#7470)
  fix(tree-item): move focus outline to item label area (#7581)
  chore: add explainers to disabled jsx-sort-props on ref prop (#7584)
  fix(chip-group): Add existence checks (#7586)
  refactor(combobox, combobox-item, combobox-group): `getElementProp` is refactored out across child components as an outdated pattern in favor of inheritable props set directly on parent (#7562)
  build(deps): update dependency eslint-plugin-react to v7.33.2 (#7565)
  build(deps): update dependency quicktype-core to v23.0.71 (#7566)
  chore: release next
  fix(date-picker): set start of week to monday in zh-CN (#7578)
  chore: update translations (#7542)
  build: update browserslist db (#7554)
  ...
benelan added a commit that referenced this pull request Aug 24, 2023
@benelan
Merge remote-tracking branch 'origin/main' into benelan/7531-pr-merge…
4a74a1a 
…-failsafe

* origin/main: (29 commits)
  chore: release next
  refactor(combobox-item): drop unused variable (#7587)
  fix(combobox): update combobox height to follow design spec (#7558)
  fix(split-button): update divider and borders to follow design spec (#7568)
  fix(segmented-control): refresh items when added dynamically (#7567)
  chore: release next
  feat(action-bar, action-pad, action-group): Add label properties for group context (#7415)
  test(modal, shell): add delay to recent unstable stories (#7589)
  chore: release next
  fix(modal): handle removal of open attribute and prevent multiple beforeClose calls (#7470)
  fix(tree-item): move focus outline to item label area (#7581)
  chore: add explainers to disabled jsx-sort-props on ref prop (#7584)
  fix(chip-group): Add existence checks (#7586)
  refactor(combobox, combobox-item, combobox-group): `getElementProp` is refactored out across child components as an outdated pattern in favor of inheritable props set directly on parent (#7562)
  build(deps): update dependency eslint-plugin-react to v7.33.2 (#7565)
  build(deps): update dependency quicktype-core to v23.0.71 (#7566)
  chore: release next
  fix(date-picker): set start of week to monday in zh-CN (#7578)
  chore: update translations (#7542)
  build: update browserslist db (#7554)
  ...
benelan pushed a commit that referenced this pull request Sep 1, 2023
@calcite-admin @github-actions
chore: release main (#7571)
784cc25 
🤖 I have created a release *beep* *boop*
---


<details><summary>@esri/calcite-components: 1.7.0</summary>

##
[1.7.0](https://github.com/Esri/calcite-design-system/compare/@esri/[email protected]...@esri/[email protected])
(2023-09-01)


### Features

* **action-bar, action-pad, action-group:** Add label properties for
group context
([#7415](#7415))
([b34f36d](b34f36d))
* **combobox:** Add single-persist selection mode
([#7583](#7583))
([dab06a3](dab06a3))
* **flow:** Add support for custom flow-item elements
([#7608](#7608))
([197adfe](197adfe))
* **input-date-picker:** Normalize year to current century for user
typed values only
([#7638](#7638))
([a1db718](a1db718))
* **input-number:** Add integer property
([#7646](#7646))
([cd66a6d](cd66a6d))
* **input-time-picker:** Support fractional seconds
([#7532](#7532))
([c2bf34b](c2bf34b))
* **meter:** Add Meter component
([#7401](#7401))
([47163ed](47163ed))
* **sheet:** Add Sheet component
([#7561](#7561))
([f12a393](f12a393))
* **sheet:** Update default widths
([#7617](#7617))
([47d2c0b](47d2c0b))
* **shell:** Add "Sheets" Slot
([#7579](#7579))
([e798765](e798765))
* **table:** Add Table and related components
([#7607](#7607))
([b067e72](b067e72))


### Bug Fixes

* **accordion, accordion-item:** Improve a11y
([#7560](#7560))
([b5170b6](b5170b6))
* Add drag styles for improved UX
([#7644](#7644))
([afbb764](afbb764))
* **block, block-section:** Improve a11y
([#7557](#7557))
([1f44f6b](1f44f6b))
* **chip-group:** Add existence checks
([#7586](#7586))
([5ca64f1](5ca64f1))
* **color-picker:** Update value when alphaChannel is toggled
([#7563](#7563))
([1f753dd](1f753dd))
* **combobox:** Prevent deselecting items via keyboard in single-persist
mode
([#7634](#7634))
([4f5f8b0](4f5f8b0))
* **combobox:** Update combobox height to follow design spec
([#7558](#7558))
([ec08845](ec08845))
* **date-picker:** Set start of week to monday in zh-CN
([#7578](#7578))
([7e385cb](7e385cb))
* **dropdown:** Prevents navigating dropdown items with Tab key
([#7527](#7527))
([3ea658d](3ea658d))
* Ensure label only focuses the first labelable child
([#7553](#7553))
([426159c](426159c))
* **flow:** Catch error when beforeBack promise is rejected
([#7601](#7601))
([cb70671](cb70671))
* **input-date-picker, input-time-picker:** Do not show dropdown
affordance when read-only
([#7559](#7559))
([5a3f19c](5a3f19c))
* **input, input-number:** Correctly sanitize numbers when pasting
string with 'e'
([#7648](#7648))
([b8bc11c](b8bc11c))
* **list, sortable-list, value-list:** Emit calciteListOrderChange when
dragging between lists
([#7614](#7614))
([4653581](4653581))
* **list:** Fixes dragging nested list items
([#7555](#7555))
([c25f7b3](c25f7b3))
* **list:** Stop emitting calciteListChange when a list-item is disabled
or closed.
([#7624](#7624))
([7008463](7008463))
* **loader:** Tweak loading animations to work in Safari
([#7564](#7564))
([2103654](2103654))
* **modal:** Catch error when beforeClose promise is rejected
([#7600](#7600))
([70405d0](70405d0))
* **modal:** Handle removal of open attribute and prevent multiple
beforeClose calls
([#7470](#7470))
([f31588f](f31588f))
* **rating:** Adds focus outline on click
([#7341](#7341))
([af30073](af30073))
* **segmented-control:** Refresh items when added dynamically
([#7567](#7567))
([2e36eb3](2e36eb3))
* **split-button:** Update divider and borders to follow design spec
([#7568](#7568))
([8df59ab](8df59ab))
* **tree-item:** Move focus outline to item label area
([#7581](#7581))
([1327cef](1327cef))
* **tree-item:** Updates state when selection changes programmatically
for `selection-mode='ancestors'`
([#7572](#7572))
([40758c5](40758c5))
* **tree:** Improve keyboard navigation
([#7618](#7618))
([826a5cb](826a5cb))
</details>

<details><summary>@esri/calcite-components-react: 1.7.0</summary>

##
[1.7.0](https://github.com/Esri/calcite-design-system/compare/@esri/[email protected]...@esri/[email protected])
(2023-09-01)


### Bug Fixes

* Make sure components are defined in environments like in codesandbox
([#7632](#7632))
([7005cce](7005cce))


### Dependencies

* The following workspace dependencies were updated
  * dependencies
    * @esri/calcite-components bumped from ^1.7.0-next.22 to ^1.7.0
</details>

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jcfranco jcfranco jcfranco approved these changes

@geospatialem geospatialem Awaiting requested review from geospatialem

Assignees

@driskull driskull

Labels
enhancement Issues tied to a new feature or request. pr ready for visual snapshots Adding this label will run visual snapshot testing.
Projects
None yet
Milestone
2023 August Priorities
Development

Successfully merging this pull request may close these issues.
3 participants
@driskull @jcfranco @geospatialem