Skip to content

Conversation

@git-nandor
Copy link
Contributor

INSTUI-4876
INSTUI-4877

Summary

Migrated List and InlineList component from the old theming system.

Test plan

On the documentation page, verify that everything displays and works correctly.

Co-Authored-By: 🤖 Claude

@git-nandor git-nandor self-assigned this Dec 15, 2025
@github-actions
Copy link

github-actions bot commented Dec 15, 2025

PR Preview Action v1.6.3

🚀 View preview at
https://instructure.design/pr-preview/pr-2308/

Built to branch gh-pages at 2025-12-19 10:24 UTC.
Preview will be ready when the GitHub Pages deployment is complete.

@git-nandor git-nandor force-pushed the INSTUI-4876_list_rework branch from 19734f7 to 16b5427 Compare December 15, 2025 16:54
@git-nandor git-nandor marked this pull request as ready for review December 15, 2025 16:55
@git-nandor git-nandor force-pushed the INSTUI-4876_list_rework branch from 16b5427 to dc40cb4 Compare December 16, 2025 08:39
Copy link
Collaborator

@adamlobler adamlobler left a comment

Choose a reason for hiding this comment

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

List items aren’t getting the proper tokens for some reason. The text color should be white in the dark theme, and the font family should change as well. Maybe they’re not using the inlineListItem / listItem CSS values. 

Image

Comment on lines 42 to 45
const generateStyle = (
componentTheme: InlineListItemTheme,
componentTheme: NewComponentTypes['InlineListInlineListItem'],
props: InlineListItemProps
): InlineListItemStyle => {
Copy link
Contributor

Choose a reason for hiding this comment

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

I see a marginBottomDefault token in InlineListInlineListItem which I don't see here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The marginBottomDefault token will not be used, and the designers have already removed it in the latest version.

@git-nandor git-nandor force-pushed the INSTUI-4876_list_rework branch from dc40cb4 to d8de7ed Compare December 17, 2025 09:16
@git-nandor
Copy link
Contributor Author

List items aren’t getting the proper tokens for some reason. The text color should be white in the dark theme, and the font family should change as well. Maybe they’re not using the inlineListItem / listItem CSS values. 

Solved. The tokens now receive the correct values.

Comment on lines +74 to +80
makeStyleProps = () => {
const { size, delimiter, spacing } = this.props
return {
size,
delimiter,
spacing
}
Copy link
Contributor

@ToMESSKa ToMESSKa Dec 18, 2025

Choose a reason for hiding this comment

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

Is there any particular reason why you're extracting props in the index.ts files?

Copy link
Contributor Author

@git-nandor git-nandor Dec 18, 2025

Choose a reason for hiding this comment

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

This ensures that only the necessary props are passed to the theme.ts file for style generation. Similar to ui-rating, I think this is a clean and effective approach.

Copy link
Contributor

@ToMESSKa ToMESSKa Dec 19, 2025

Choose a reason for hiding this comment

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

@git-nandor I checked it and I cannot see a smiliar approach in ui-rating?

@git-nandor git-nandor force-pushed the INSTUI-4876_list_rework branch 2 times, most recently from 0be30e8 to 9abad94 Compare December 19, 2025 08:42
@git-nandor git-nandor requested a review from ToMESSKa December 19, 2025 08:42
@git-nandor git-nandor force-pushed the INSTUI-4876_list_rework branch from 9abad94 to e13556a Compare December 19, 2025 10:19
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.

4 participants