Skip to content

feat(list-item): introduce new styles#1949

Open
ljanner wants to merge 1 commit intomainfrom
feat/list-item
Open

feat(list-item): introduce new styles#1949
ljanner wants to merge 1 commit intomainfrom
feat/list-item

Conversation

@ljanner
Copy link
Copy Markdown
Member

@ljanner ljanner commented Apr 23, 2026

@ljanner ljanner added this to the 49.x milestone Apr 23, 2026
@ljanner ljanner self-assigned this Apr 23, 2026
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces the List Item component, including its documentation, SCSS styles, and interactive examples. The feedback highlights several areas for improvement: removing placeholder suffixes in documentation headings, increasing CSS selector specificity in the theme styles, and adhering to UX writing guidelines regarding timestamp vocabulary, telegram-style descriptions, button placement, and label capitalization.

Comment thread docs/components/lists-tables-trees/list-item.md
Comment thread projects/element-theme/src/styles/components/_list-item.scss Outdated
Comment thread src/app/examples/list-item/list-item-action.html
Comment thread src/app/examples/list-item/list-item.html
Comment thread src/app/examples/list-item/list-item.html
@ljanner ljanner force-pushed the feat/list-item branch 3 times, most recently from c072124 to 8d5328e Compare April 23, 2026 15:06
@ljanner
Copy link
Copy Markdown
Member Author

ljanner commented Apr 23, 2026

@spike-rabbit could you take a look at the example and provide some initial feedback if the structure looks as expected?

@ljanner ljanner requested a review from spike-rabbit April 23, 2026 15:24
Comment thread projects/element-theme/src/styles/components/_list-item.scss Outdated
Comment thread projects/element-theme/src/styles/components/_list-item.scss Outdated
Comment thread projects/element-theme/src/styles/components/_list-item.scss Outdated
Copy link
Copy Markdown
Member

@panch1739 panch1739 left a comment

Choose a reason for hiding this comment

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

@ljanner Looks great!

Just one observation...in the list-item-actions, the active/selected state seems to be missing. When the list item is pressed, the background color should change to base-1-selected.

Questions:

  • The replacement of the notification item would happen in another PR?
  • Does this have the "unread" property also? (This is only valid for notification item)

@ljanner ljanner force-pushed the feat/list-item branch 2 times, most recently from f64852f to 7ab5d4b Compare April 29, 2026 14:57
@ljanner
Copy link
Copy Markdown
Member Author

ljanner commented Apr 29, 2026

@panch1739

  • Does this have the "unread" property also? (This is only valid for notification item)

I now added a new example called list-item-unread. While an unread state isn't quite generic I do think it makes sense to provide it since the idea is to drop the notification item entirely.

The replacement of the notification item would happen in another PR?

For the replacement itself, we need to choose a technical solution either:

  • Deprecate the notification item directly in this PR
  • Rebuild it internally to use list-item (though the effort may not be worth it)

This is still open, we can see how far we can get with migrations for element v50.

For now I added a banner on the notification item docs recommending list-item instead.

@ljanner ljanner marked this pull request as ready for review April 30, 2026 11:00
@ljanner ljanner requested review from a team as code owners April 30, 2026 11:00
@ljanner ljanner requested a review from panch1739 April 30, 2026 11:00
@panch1739
Copy link
Copy Markdown
Member

@panch1739

  • Does this have the "unread" property also? (This is only valid for notification item)

I now added a new example called list-item-unread. While an unread state isn't quite generic I do think it makes sense to provide it since the idea is to drop the notification item entirely.

The replacement of the notification item would happen in another PR?

For the replacement itself, we need to choose a technical solution either:

  • Deprecate the notification item directly in this PR
  • Rebuild it internally to use list-item (though the effort may not be worth it)

This is still open, we can see how far we can get with migrations for element v50.

For now I added a banner on the notification item docs recommending list-item instead.

@ljanner Excellent! Is good for me :) Thank you very much!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Transform notification item in a generic list item

4 participants