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(combobox): add space after grouped items #7302

Merged

Conversation

Elijbet
Copy link
Contributor

@Elijbet Elijbet commented Jul 9, 2023

Related Issue: #6412

Summary

Adding space at the bottom of grouped items to provide visual separation between the group and any following content.

  • Bottom padding added to combobox-item-group at all scales.
  • Added padding would not be present if the group has no items, allowing for the use of only the group heading (with no extra padding) when nesting groups.
CleanShot 2023-07-10 at 09 19 44@2x

@github-actions github-actions bot added the bug Bug reports for broken functionality. Issues should include a reproduction of the bug. label Jul 9, 2023
@Elijbet Elijbet marked this pull request as ready for review July 9, 2023 23:43
@Elijbet Elijbet requested a review from a team as a code owner July 9, 2023 23:43
@Elijbet Elijbet added the pr ready for visual snapshots Adding this label will run visual snapshot testing. label Jul 9, 2023
@Elijbet Elijbet 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 Jul 9, 2023
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.

This looks good. Should ancestors be marked as @internal in doc? We don't want or need users to set ancestors right? cc @geospatialem @jcfranco

Copy link
Contributor

@anveshmekala anveshmekala left a comment

Choose a reason for hiding this comment

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

🙌

Other than naming everything looks great!

@jcfranco
Copy link
Member

Should ancestors be marked as @internal in doc? We don't want or need users to set ancestors right?

Yes, it should be internal. Nice catch. Let's do this separately to confirm that using @internal only removes it from the doc (i.e., prop and associated type are unaffected). Otherwise, this would fall under breaking change.

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.

👍

@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 Jul 19, 2023
@Elijbet Elijbet removed Stale Issues or pull requests that have not had recent activity. pr ready for visual snapshots Adding this label will run visual snapshot testing. labels Jul 25, 2023
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!

👾👾👾👾👾👾👾👾👾👾👾👾👾👾👾👾👾👾👾👾👾👾👾👾👾👾👾👾👾👾👾👾👾👾👾👾👾👾👾👾
👾👩‍🚀👩‍🚀👩‍🚀👩‍🚀👾👩‍🚀👾👾👾👾👾👩‍🚀👾👩‍🚀👩‍🚀👩‍🚀👩‍🚀👾👾👩‍🚀👩‍🚀👩‍🚀👾👾👩‍🚀👩‍🚀👾👾👩‍🚀👾👾👾👩‍🚀👾👩‍🚀👩‍🚀👩‍🚀👩‍🚀👾
👾👩‍🚀👾👾👩‍🚀👾👩‍🚀👾👾👾👾👾👩‍🚀👾👩‍🚀👾👾👾👾👩‍🚀👾👾👾👾👩‍🚀👾👾👩‍🚀👾👩‍🚀👩‍🚀👾👩‍🚀👩‍🚀👾👩‍🚀👾👾👾👾
👾👩‍🚀👩‍🚀👩‍🚀👩‍🚀👾👩‍🚀👾👾👩‍🚀👾👾👩‍🚀👾👩‍🚀👩‍🚀👩‍🚀👾👾👾👩‍🚀👩‍🚀👾👾👩‍🚀👾👾👩‍🚀👾👩‍🚀👾👩‍🚀👾👩‍🚀👾👩‍🚀👩‍🚀👩‍🚀👾👾
👾👩‍🚀👾👾👩‍🚀👾👩‍🚀👾👩‍🚀👾👩‍🚀👾👩‍🚀👾👩‍🚀👾👾👾👾👾👾👾👩‍🚀👾👩‍🚀👾👾👩‍🚀👾👩‍🚀👾👾👾👩‍🚀👾👩‍🚀👾👾👾👾
👾👩‍🚀👾👾👩‍🚀👾👾👩‍🚀👾👾👾👩‍🚀👾👾👩‍🚀👩‍🚀👩‍🚀👩‍🚀👾👩‍🚀👩‍🚀👩‍🚀👾👾👾👩‍🚀👩‍🚀👾👾👩‍🚀👾👾👾👩‍🚀👾👩‍🚀👩‍🚀👩‍🚀👩‍🚀👾
👾👾👾👾👾👾👾👾👾👾👾👾👾👾👾👾👾👾👾👾👾👾👾👾👾👾👾👾👾👾👾👾👾👾👾👾👾👾👾👾

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.

LGTM! 👏

Elijbet added 3 commits August 2, 2023 19:25
… codebase setup, target CSS.groupThatFollowsChildlessGroup directly instead and rework the solution to be able to override that one group
@Elijbet Elijbet added the pr ready for visual snapshots Adding this label will run visual snapshot testing. label Aug 3, 2023
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! 🎉

@jcfranco jcfranco changed the title fix(combobox-item-group): add space after grouped items fix(combobox): add space after grouped items Aug 3, 2023
@Elijbet Elijbet 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 3, 2023
@benelan benelan merged commit b1580c7 into main Aug 3, 2023
@benelan benelan deleted the elijbet/6412-combobox-item-group-add-space-after-grouped-items branch August 3, 2023 18:34
@github-actions github-actions bot added this to the 2023 July Priorities milestone Aug 3, 2023
benelan added a commit that referenced this pull request Aug 3, 2023
* origin/main: (40 commits)
  fix(combobox): add space after grouped items (#7302)
  fix(tooltip): emits `close` and `beforeClose` events when container is set to `display:none` (#7258)
  chore: release next
  feat(block): improve block's content layout to allow scrolling (#7367)
  test(input, input-number): await on missed `page.waitForChanges` calls (#7429)
  chore: release next
  fix(color-picker): draw slider thumbs within bounds (#7398)
  chore: release next
  fix(slider): prevent excessive tick rendering (#7421)
  fix(tooltip): avoid extra before open/close event emitting (#7422)
  chore(storybook): hide warning and show TestOnly stories in local builds (#7424)
  chore: release next
  fix(panel): Remove double border styling when content isn't provided (#7368)
  feat(list): Add support for dragging items. (#7109)
  chore: release next
  fix(scrim): update loader scale on resize of component. (#7419)
  feat(text-area): provide additional context for AT users when character limit exceeds (#7412)
  chore: release next
  fix(chip): disconnect mutation observer when component is disconnected from the DOM (#7418)
  fix(switch): Fix for focus outline style in certain cases (#7414)
  ...
benelan pushed a commit that referenced this pull request Aug 3, 2023
🤖 I have created a release *beep* *boop*
---


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

##
[1.5.0](https://github.com/Esri/calcite-design-system/compare/@esri/[email protected]...@esri/[email protected])
(2023-08-03)


### Features

* **action-group:** Adds overlayPositioning property.
([#7366](#7366))
([ca9f35a](ca9f35a))
* Allow sharing focus trap stacks via configuration global
([#7334](#7334))
([934a19f](934a19f))
* Automatically import and define Calcite Components when importing
their React wrapper
([#7185](#7185))
([bf0ff67](bf0ff67))
* **block, block-section:** Add setFocus method
([#7208](#7208))
([35d4bbb](35d4bbb))
* **block:** Improve block's content layout to allow scrolling
([#7367](#7367))
([ecbf17b](ecbf17b))
* **color-picker:** Replaces thumb focus outline to rounded
([#7378](#7378))
([d803980](d803980))
* **filter:** Add filter method
([#7127](#7127))
([5a4283f](5a4283f))
* **flow:** Adds setFocus method
([#7252](#7252))
([2472c58](2472c58))
* Improve focus behavior in components
([#7277](#7277))
([ad9fbca](ad9fbca))
* **input-time-zone:** Add input-time-zone component
([#6947](#6947))
([87bd496](87bd496))
* **list:** Add slots for filter actions
([#7183](#7183))
([da07ab1](da07ab1))
* **list:** Add support for dragging items.
([#7109](#7109))
([7324f70](7324f70))
* **menu-item:** Update spacing and icon layout
([#7381](#7381))
([5659671](5659671))
* **navigation-logo:** Increase font-size of heading with no description
([#7081](#7081))
([355e101](355e101))
* **switch:** Updates focus outline to be rounded
([#7390](#7390))
([2616b82](2616b82))
* **text-area:** Provide additional context for AT users when character
limit exceeds
([#7299](#7299))
([c5678eb](c5678eb))
* **text-area:** Provide additional context for AT users when character
limit exceeds
([#7412](#7412))
([c1af3c7](c1af3c7))


### Bug Fixes

* **accordion, accordion-item:** `icon-position`, `icon-type`,
`selection-mode` and `scale` can now be set as props or attributes
([#7191](#7191))
([2b09aba](2b09aba))
* **action-bar:** No longer delegates focus when clicked on
non-focusable region
([#7310](#7310))
([1a9c15c](1a9c15c))
* **action:** Correctly focus the button after rendering updates.
([#7255](#7255))
([40fe2ce](40fe2ce))
* **block:** Loader now appears for all loading cases
([#7303](#7303))
([5af3600](5af3600))
* **block:** Removes extra loading indicator
([#7239](#7239))
([a334a75](a334a75))
* **card:** Ensure teardown logic is called when disconnected
([#7289](#7289))
([d07e322](d07e322))
* **chip:** Disconnect mutation observer when component is disconnected
from the DOM
([#7418](#7418))
([412e5fb](412e5fb))
* **color-picker:** Draw slider thumbs within bounds
([#7398](#7398))
([2f37854](2f37854))
* **color-picker:** Fix opacity slider keyboard nudging
([#7400](#7400))
([2b4f7c3](2b4f7c3))
* **color-picker:** Maintains correct numbering system when entering
invalid RGB value
([#7327](#7327))
([8d2a3a5](8d2a3a5))
* **combobox:** Add space after grouped items
([#7302](#7302))
([b1580c7](b1580c7))
* **dropdown-item:** Provides accessible label when href is not parsed
([#7316](#7316))
([966b83d](966b83d))
* **flow:** Call setFocus() on back button click
([#7285](#7285))
([9102aa4](9102aa4))
* **input-date-picker:** Provides placeholder text context for AT users
([#7320](#7320))
([31e0ba2](31e0ba2))
* **input-date-picker:** Reset active date picker date after closing
([#7219](#7219))
([91b2a1b](91b2a1b))
* **input, input-number:** No longer removes trailing decimal separator
([#7159](#7159))
([01535cf](01535cf))
* **link:** Adds outline-offset to avoid overlapping with text.
([#7342](#7342))
([c30db4e](c30db4e))
* **list:** Changing filterText property will now update filtered items
([#7133](#7133))
([a9c0bce](a9c0bce))
* **list:** Fix keyboard navigation after a listItem's disabled or
closed property changes
([#7275](#7275))
([91d28eb](91d28eb))
* **list:** Fix keyboard navigation when filterEnabled is true
([#7385](#7385))
([41a2e42](41a2e42))
* **menu-item:** Prevent duplicate border in nested vertical menu-items
([#7387](#7387))
([186a738](186a738))
* **panel:** Remove double border styling when content isn't provided
([#7368](#7368))
([91a0610](91a0610))
* Remove style modifying all host components with hidden attribute
([#7346](#7346))
([3103e2f](3103e2f))
* **scrim:** Update loader scale on resize of component.
([#7419](#7419))
([24e7f70](24e7f70))
* **slider:** Prevent excessive tick rendering
([#7421](#7421))
([c799409](c799409))
* **switch:** Fix for focus outline style in certain cases
([#7414](#7414))
([217324f](217324f))
* **tab-title:** Add full focus outline to closable tab button in high
contrast mode
([#7272](#7272))
([d812d17](d812d17))
* **tooltip:** Avoid extra before open/close event emitting
([#7422](#7422))
([dbb6818](dbb6818))
* **tooltip:** Deprecate the label property due to the description
coming from the component's content
([#7247](#7247))
([7934d75](7934d75))
* **tooltip:** Emits `close` and `beforeClose` events when container is
set to `display:none`
([#7258](#7258))
([60a4683](60a4683))
* **tooltip:** Ensure --calcite-app-z-index-tooltip is applied
([#7345](#7345))
([a9a7072](a9a7072))
</details>

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

##
[1.5.0](https://github.com/Esri/calcite-design-system/compare/@esri/[email protected]...@esri/[email protected])
(2023-08-03)


### Features

* Automatically import and define Calcite Components when importing
their React wrapper
([#7185](#7185))
([bf0ff67](bf0ff67))


### Dependencies

* The following workspace dependencies were updated
  * dependencies
    * @esri/calcite-components bumped from ^1.5.0-next.38 to ^1.5.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
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.

5 participants