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

fix(list, list-item, list-item-group): improve drag experience by indenting items #9169

Merged
merged 13 commits into from
Apr 25, 2024
Merged

fix(list, list-item, list-item-group): improve drag experience by indenting items #9169

merged 13 commits into from
Apr 25, 2024

Conversation

driskull
Copy link
Member

@driskull driskull commented Apr 22, 2024 •
edited
Loading

Related Issue: #8242 #8667 #9058

Summary

  • Properly indents each list-item nested within another list or list-item
  • Removes transparent bg from handle, not necessary
  • Updates internal table cell classes to remove padding being set by default browser styles.
  • Changes borders to use proper css borders instead of a 1px margin and shadow
  • Sets bg color of foreground 1 on list-item.
  • Stretch action and handles slotted within a list-item.
  • Removes padding from empty lists
  • Remove unnecessary utils from list
  • Add and improve screenshot tests

@github-actions github-actions bot added the bug Bug reports for broken functionality. Issues should include a reproduction of the bug. label Apr 22, 2024
@driskull driskull added the pr ready for visual snapshots Adding this label will run visual snapshot testing. label Apr 22, 2024
@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 Apr 22, 2024
@driskull driskull marked this pull request as ready for review April 22, 2024 23:33
@driskull driskull requested a review from a team as a code owner April 22, 2024 23:33
driskull added 2 commits April 23, 2024 13:43
@driskull
move bottom content out of indentation.
9a62554
@driskull
Merge branch 'main' into dris0000/list-indent-fix
09e6e0e 
# Conflicts:
#	packages/calcite-components/src/components/list-item-group/list-item-group.scss
@alisonailea alisonailea self-requested a review April 23, 2024 20:59
alisonailea
alisonailea reviewed Apr 23, 2024
View reviewed changes
Copy link
Contributor

@alisonailea alisonailea left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I'd like to hear @ashetland thoughts on this but this interaction still feels clunky.

You'll see in the gif Hi!5 easily slots up to become a nested child of Hi!3 but I have to drag it above Hi!2 and drag it back down to make it a child of Hi!2.

2024-04-23 14 04 23

driskull
driskull commented Apr 23, 2024
View reviewed changes
packages/calcite-components/src/components/list-item-group/list-item-group.scss Outdated
::slotted(calcite-list-item:nth-child(1 of :not([hidden]))) {
@apply shadow-none;
margin-block-start: 0px;
::slotted(calcite-list-item:not([filter-hidden], [closed]):nth-of-type(1)) {
Copy link
Member Author

Choose a reason for hiding this comment

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

@ashetland because of how this selector works, applying the border to the bottom instead of the top is going to require some refactoring.

With CSS, its much easier to get the first element but getting the last one that isn't hidden by the filter or closed is kind of not doable.

We would need to refactor to remove the last item's border with JS.

@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 Apr 23, 2024
@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 Apr 23, 2024
@ashetland
Copy link
Contributor

I'd like to hear @ashetland thoughts on this but this interaction still feels clunky.

You'll see in the gif Hi!5 easily slots up to become a nested child of Hi!3 but I have to drag it above Hi!2 and drag it back down to make it a child of Hi!2.

2024-04-23 14 04 23 2024-04-23 14 04 23

Agreed it is still a little clunky, but this is an improvement for issue #8242. We have additional improvements that we plan to address separately.

@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 Apr 23, 2024
@driskull
Copy link
Member Author

There's still some room for improvement for sure. We need to research to see why dragging into a child isn't as easily triggered as it should be. However, from a visual perspective at least now you can see which depth level you are moving into whereas previously you couldn't tell at all.

@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 Apr 23, 2024
@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 Apr 24, 2024
@driskull driskull requested a review from jcfranco April 24, 2024 21:08
@driskull
Copy link
Member Author

@jcfranco can you review this? I think its good to go after.

jcfranco
jcfranco approved these changes Apr 25, 2024
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 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 Apr 25, 2024
@driskull driskull merged commit 88aab49 into main Apr 25, 2024
16 checks passed
@driskull driskull deleted the dris0000/list-indent-fix branch April 25, 2024 18:32
@github-actions github-actions bot added this to the 2024-04-30 - Apr Release milestone Apr 25, 2024
@calcite-admin calcite-admin mentioned this pull request Apr 25, 2024
Merged
geospatialem pushed a commit that referenced this pull request May 1, 2024
@calcite-admin @github-actions
chore: release main (#9050)
1ad530e 
🤖 I have created a release *beep* *boop*
---


<details><summary>@esri/calcite-design-tokens: 2.2.0</summary>

##
[2.2.0](https://github.com/Esri/calcite-design-system/compare/@esri/calcite-design-tokens@2.1.2...@esri/calcite-design-tokens@2.2.0)
(2024-04-30)


### Features

* Ensure all components inherit font-family
([#8388](#8388))
([90f8923](90f8923))
</details>

<details><summary>@esri/eslint-plugin-calcite-components:
1.2.0</summary>

##
[1.2.0](https://github.com/Esri/calcite-design-system/compare/@esri/eslint-plugin-calcite-components@1.1.0...@esri/eslint-plugin-calcite-components@1.2.0)
(2024-04-30)


### Features

* **enforce-ref-last-prop:** Add fixer
([#8671](#8671))
([688bc51](688bc51))


### Bug Fixes

* **slider:** Improve snapping + step logic
([#8169](#8169))
([8b83042](8b83042))
</details>

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

##
[2.8.0](https://github.com/Esri/calcite-design-system/compare/@esri/calcite-components@2.7.1...@esri/calcite-components@2.8.0)
(2024-04-30)


### Features

* Add calciteInvalid event for form validation
([#9211](#9211))
([a504508](a504508))
* Add validity property to form components
([#9210](#9210))
([e49902b](e49902b))
* **carousel:** Add Carousel and Carousel Item components
([#8995](#8995))
([b1ba428](b1ba428))
* **enforce-ref-last-prop:** Add fixer
([#8671](#8671))
([688bc51](688bc51))
* Ensure all components inherit font-family
([#8388](#8388))
([90f8923](90f8923))
* **input-time-zone:** Add readonly support
([#9111](#9111))
([153a122](153a122))
* **input-time-zone:** Allow clearing value
([#9168](#9168))
([193bb7d](193bb7d))
* **list-item-group:** Update list item group styles
([#9072](#9072))
([c734849](c734849))
* **navigation-logo:** Add heading level
([#9054](#9054))
([c2beba8](c2beba8))
* **pagination:** Add navigation methods
([#9154](#9154))
([5ca6a5f](5ca6a5f))
* **slider:** Add support for custom label formatting
([#9179](#9179))
([710d1ee](710d1ee))
* **slider:** Allow configuring fill behavior
([#9170](#9170))
([1b2cdbf](1b2cdbf))
* **tile, tile-group:** Support single, multi, single-persist, none
selection modes and icon and border selection appearances
([#9159](#9159))
([2d77470](2d77470))


### Bug Fixes

* **action:** Maintain equal height when text is not enabled in a small
scale
([#9051](#9051))
([407824a](407824a))
* **chip-group:** Improve programmatic Chip selection behavior
([#9213](#9213))
([b7a4c77](b7a4c77))
* **combobox:** Update the focus and hover chevron icon color
([#9187](#9187))
([a1317da](a1317da))
* **combobox, input-time-zone:** Improve readOnly behavior and styling
([#9222](#9222))
([606d80f](606d80f))
* **combobox:** Fix aria-role for proper voiceover support
([#9039](#9039))
([eebe8ca](eebe8ca))
* **combobox:** Update the focused chevron icon color
([#9202](#9202))
([27ac02d](27ac02d))
* **date-picker:** Ensure `proximitySelectionDisabled` resets range on
post-range-commit selection
([#9171](#9171))
([f466b14](f466b14))
* **date-picker:** Restore focus on date when navigating month with
arrow/page keys
([#9063](#9063))
([db109e0](db109e0))
* Fix memory leak affecting components using conditionally-rendered
slots
([#9208](#9208))
([28701b6](28701b6))
* **input-date-picker:** Update the focus and hover chevron icon color
([#9146](#9146))
([ca895e9](ca895e9))
* **input-time-zone:** Ensure selected item is properly displayed when
there are other items with the same offset
([#9134](#9134))
([1f94903](1f94903))
* **input, input-number, input-text, input-date-picker,
input-time-picker, filter, menu-item:** Ignore canceled events and
enforce cancellations where needed
([#8952](#8952))
([d0fa977](d0fa977))
* **list, list-item, list-item-group:** Improve drag experience by
indenting items
([#9169](#9169))
([88aab49](88aab49))
* **list, list-item:** Support keyboard sorting in screen readers
([#9038](#9038))
([b2e1b9b](b2e1b9b))
* **list:** Closed list-items no longer render extra space
([#9031](#9031))
([3985004](3985004))
* **panel:** Support cancelling the esc key event to prevent closing a
panel
([#9032](#9032))
([ecfa5f1](ecfa5f1))
* **preset:** Update the focus outline color
([#9098](#9098))
([725f47c](725f47c))
* **radio-button:** Display validation message in parent group
([#9218](#9218))
([b1e869f](b1e869f))
* **select:** Update the focus and hover chevron icon color and select
focus and hover background color
([#9160](#9160))
([b187340](b187340))
* **slider:** Fix fill placement and tick styling when ranged
([#9204](#9204))
([a84d831](a84d831))
* **slider:** Improve snapping + step logic
([#8169](#8169))
([8b83042](8b83042))
* **slider:** Rename `highlightPlacement` to `fillPlacement`
([#9195](#9195))
([9e5a713](9e5a713))
* **slider:** Style ticks according to the fill placement
([#9196](#9196))
([5eb4e10](5eb4e10))
* **stepper-item:** Remove delay in highlighting item
([#8996](#8996))
([bcf23dd](bcf23dd))
* **tree-item:** Do not select when chevron clicked
([#9115](#9115))
([99ad8f1](99ad8f1))


### Dependencies

* The following workspace dependencies were updated
  * devDependencies
    * @esri/calcite-design-tokens bumped from ^2.2.0-next.0 to ^2.2.0
* @esri/eslint-plugin-calcite-components bumped from ^1.2.0-next.1 to
^1.2.0
</details>

<details><summary>@esri/calcite-components-angular: 2.8.0</summary>

##
[2.8.0](https://github.com/Esri/calcite-design-system/compare/@esri/calcite-components-angular@2.7.1...@esri/calcite-components-angular@2.8.0)
(2024-04-30)


### Miscellaneous Chores

* **@esri/calcite-components-angular:** Synchronize components versions


### Dependencies

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

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

##
[2.8.0](https://github.com/Esri/calcite-design-system/compare/@esri/calcite-components-react@2.7.1...@esri/calcite-components-react@2.8.0)
(2024-04-30)


### Miscellaneous Chores

* **@esri/calcite-components-react:** Synchronize components versions


### Dependencies

* The following workspace dependencies were updated
  * dependencies
    * @esri/calcite-components bumped from ^2.8.0-next.20 to ^2.8.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>
@ashetland ashetland mentioned this pull request May 3, 2024
Open
@calcite-admin calcite-admin mentioned this pull request May 14, 2024
Merged
@calcite-admin calcite-admin mentioned this pull request May 28, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alisonailea alisonailea alisonailea approved these changes

@jcfranco jcfranco jcfranco approved these changes

Assignees

@driskull driskull

@ashetland ashetland

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
Milestone
2024-04-30 - Apr Release
Development

Successfully merging this pull request may close these issues.
4 participants
@driskull @ashetland @jcfranco @alisonailea