Skip to content

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

Open
ljanner wants to merge 2 commits into
mainfrom
feat/list-item
Open

feat(list-item): introduce new styles#1949
ljanner wants to merge 2 commits into
mainfrom
feat/list-item

Conversation

@ljanner

@ljanner ljanner commented Apr 23, 2026

Copy link
Copy Markdown
Member

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

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

Copy link
Copy Markdown
Contributor

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 Outdated
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

ljanner commented Apr 23, 2026

Copy link
Copy Markdown
Member Author

@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

@panch1739 panch1739 left a comment

Copy link
Copy Markdown
Member

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

ljanner commented Apr 29, 2026

Copy link
Copy Markdown
Member Author

@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!

Comment thread src/app/examples/list-item/list-item-action.html Outdated
Comment thread src/app/examples/list-item/list-item-action.html Outdated
Comment thread src/app/examples/list-item/list-item-action.html Outdated
Comment thread src/app/examples/list-item/list-item-action.html Outdated
Comment thread src/app/examples/list-item/list-item-action.html Outdated
@ljanner ljanner force-pushed the feat/list-item branch 4 times, most recently from 1aac650 to f75d501 Compare June 3, 2026 16:35
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
@ljanner ljanner requested a review from dr-itz June 4, 2026 06:56
@ljanner ljanner dismissed chintankavathia’s stale review June 20, 2026 09:34

review status of pr seems bugged

Comment thread projects/element-theme/src/styles/components/_list-item.scss
Comment thread src/app/examples/list-item/list-item-action.html Outdated
@ljanner ljanner requested a review from spike-rabbit June 25, 2026 14:57
Comment thread src/app/examples/list-item/list-item-unread.ts Outdated
Comment thread src/app/examples/list-item/list-item.html Outdated
Comment thread src/app/examples/list-item/list-item-unread.html
Comment thread src/app/examples/list-item/list-item.ts Outdated
Comment thread src/app/examples/list-item/list-item-unread.ts Outdated
Comment thread src/app/examples/list-item/list-item-action.ts Outdated
ljanner added 2 commits July 2, 2026 15:52
DEPRECATED: `SiNotificationItemComponent` is deprecated.

Use the CSS-based list item component instead. Migrate by replacing
`<si-notification-item>` with the `.list-item` CSS classes applied to
semantic HTML elements. See the list item documentation for usage
examples and accessibility guidance.
@ljanner ljanner dismissed spike-rabbit’s stale review July 2, 2026 15:29

outdated review

<li class="list-item">
<time class="list-item-timestamp" datetime="09:30">Today at 09:30</time>
<h5 class="list-item-title">Firmware update pending approval</h5>
<div class="list-item-primary-action">

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I am not exactly in favor of the name primary-action.
The UX specs only show a menu there. So what about calling it list-item-menu?

@spliffone spliffone Jul 2, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I would go something like context-action, the position is at the end and its doesn't need to be a menu it could also be on or more action buttons.

Just a side question, should we shorten the naming?
.list-item:

  • .title
  • .timestamp
  • .description
  • .metadata

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The naming here is difficult and has previously also been a discussion for the notification-item. Here is the link to the full discussion (internal): https://code.siemens.com/simpl/simpl-element/-/work_items/2371#note_39626788

I also think list-item-menu is too specific as other action buttons are totally valid. To me personally the name context-action lacks hierarchal meaning.

I'm open to something like list-item-main-action or list-item-inline-action.

/cc @kfenner

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

6 participants