Skip to content

Hotfix: Do not set icon color if item is active#17703

Merged
iOvergaard merged 1 commit intorelease/15.1from
15.1/hotfix/active-menu-item-icon-color
Dec 4, 2024
Merged

Hotfix: Do not set icon color if item is active#17703
iOvergaard merged 1 commit intorelease/15.1from
15.1/hotfix/active-menu-item-icon-color

Conversation

@madsrasmussen
Copy link
Member

Fixes: #16882

The fix has been applied to tree items in general, media, and document tree items.

@iOvergaard iOvergaard merged commit 9f9aaa6 into release/15.1 Dec 4, 2024
@iOvergaard iOvergaard deleted the 15.1/hotfix/active-menu-item-icon-color branch December 4, 2024 10:01

override renderIconContainer() {
const icon = this.item?.documentType.icon;
const iconWithoutColor = icon?.split(' ')[0];
Copy link
Member

Choose a reason for hiding this comment

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

The icon?.split(' ')[0]; logic is shared, should it be a method on the base class that can handle the logic?

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 would prefer if this got moved all the way to the server. If that is not possible, we should map the data when we receive it. Instead of a string with both, we should have name and color in separate props. I think we will need this in more cases.

Copy link
Member

Choose a reason for hiding this comment

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

To me it seems like a UI concern that a picked icon and colour has to change, but I can shared your idea with the rest of the team

Copy link
Member

Choose a reason for hiding this comment

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

Otherwise you only talk about splitting colour and icon

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants