Skip to content

feat(list)!: add displayMode property to choose between flat and nested lists #10852

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
Merged
merged 8 commits into from
Nov 25, 2024
Merged

feat(list)!: add displayMode property to choose between flat and nested lists #10852

merged 8 commits into from
Nov 25, 2024

Conversation

driskull
Copy link
Member

@driskull driskull commented Nov 23, 2024 •
edited
Loading

Related Issue: #9173

Summary

  • add displayMode property to choose between flat and nested lists
  • update demos
  • update stories
  • add tests

BREAKING CHANGE: Choose a displayMode of "nested" or "flat" according to space and nesting requirements.

@driskull
Copy link
Member Author

@ashetland could you give this a view? 👀

@github-actions github-actions bot added the enhancement Issues tied to a new feature or request. label Nov 23, 2024
@driskull driskull added the pr ready for visual snapshots Adding this label will run visual snapshot testing. label Nov 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 Nov 23, 2024
@driskull driskull requested a review from ashetland November 23, 2024 01:38
@driskull driskull changed the title feat(list): add mode property to choose between flat and nested lists feat(list)!: add mode property to choose between flat and nested lists Nov 23, 2024
@driskull driskull marked this pull request as ready for review November 25, 2024 18:22
macandcheese
macandcheese reviewed Nov 25, 2024
View reviewed changes
packages/calcite-components/src/components/list/list.tsx Outdated
@@ -239,6 +241,9 @@ export class List
*/
messages = useT9n<typeof T9nStrings>({ blocking: true });

/** Specifies the nesting behavior. */
@property({ reflect: true }) mode: ListMode = "flat";
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be displayMode to match some other component nomenclature? This doesn't affect any selection or other functionality right, just visual?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll defer to @jcfranco

Copy link
Contributor

Choose a reason for hiding this comment

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

It does affect some functionality. nested must be used to add the expand button and indent children. That said, I could go either way here and also defer to @jcfranco.

Copy link
Member

Choose a reason for hiding this comment

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

Good point. Let's go with displayMode for consistency. The only other mode prop I found was input-time-zone's, which affects both the API and presentation.

ashetland
ashetland approved these changes Nov 25, 2024
View reviewed changes
Copy link
Contributor

@ashetland ashetland left a comment

Choose a reason for hiding this comment

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

Visuals lookin' good!

@driskull driskull requested a review from jcfranco November 25, 2024 19:14
jcfranco
jcfranco approved these changes Nov 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.

Awesome stuff, @driskull! ✨📝✨

I have a concern regarding support for unexpected, slotted items, but aside from that, this LGTM!

packages/calcite-components/src/components/list/list.e2e.ts Outdated
</calcite-list>
</calcite-list-item>
<calcite-list-item label="Depth 2" description="Item 5"></calcite-list-item>
<div>
Copy link
Member

Choose a reason for hiding this comment

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

What's the use case for wrapped list items? If it's not a strong one, I'd suggest not supporting this to keep things simple. This also aligns with the default slot's description where we only expect items and groups. Also applies to similar dragHandle test.

Copy link
Member Author

Choose a reason for hiding this comment

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

This is basically a copy from an existing test for dragHandle. I can remove the test though.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, let's remove.

packages/calcite-components/src/components/list/list.tsx Outdated
@@ -239,6 +241,9 @@ export class List
*/
messages = useT9n<typeof T9nStrings>({ blocking: true });

/** Specifies the nesting behavior. */
@property({ reflect: true }) mode: ListMode = "flat";
Copy link
Member

Choose a reason for hiding this comment

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

Good point. Let's go with displayMode for consistency. The only other mode prop I found was input-time-zone's, which affects both the API and presentation.

@driskull driskull changed the title feat(list)!: add mode property to choose between flat and nested lists feat(list)!: add displayMode property to choose between flat and nested lists Nov 25, 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 Nov 25, 2024
@driskull driskull merged commit 832f2c9 into dev Nov 25, 2024
13 checks passed
@driskull driskull deleted the dris0000/list-mode-9173 branch November 25, 2024 21:23
This was referenced Dec 3, 2024
Closed
Closed
benelan pushed a commit that referenced this pull request Feb 8, 2025
@driskull @benelan
feat(list)!: add displayMode property to choose between flat and nest…
41283df 
…ed lists (#10852)

**Related Issue:** #9173

## Summary

- add `displayMode` property to choose between flat and nested lists
- update demos
- update stories
- add tests

BREAKING CHANGE: Choose a `displayMode` of "nested" or "flat" according
to space and nesting requirements.
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

@macandcheese macandcheese macandcheese left review comments

@ashetland ashetland ashetland approved these changes

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
2024-11-26 - 3.0.0 Nov Breaking Chang...
Development

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