Skip to content
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.

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

[message] Link color doesn't match message color #3017

Open
agjohnson opened this issue Mar 14, 2024 · 6 comments
Open

[message] Link color doesn't match message color #3017

agjohnson opened this issue Mar 14, 2024 · 6 comments
Labels
type/discussion Anything which is up for discussion type/usage Any support issues asking for help

Comments

@agjohnson
Copy link

Bug Report

All of the message variants use the default link colors, including positive/negative/warning/etc and all of the colored variants. I'm using 2.9.3.

Steps to reproduce

You can see this if you add a link inside a colored message:

<div class="ui red message">
  This is some red text.
  <a href="#">But this is the default link blue.</a>
</div>

image

Expected result

This does feel somewhat intentional, so maybe this isn't a strong issue. But I would expect the link colors to match the underlying text node colors more.

If I fix this for my own theme, I will likely use the text node color, darkened, for the link base color and use underline to denote the link:

image

I don't have strong opinions here, this might need some guidance. I'm happy to contribute this fix either way.

@agjohnson agjohnson added state/awaiting-investigation Anything which needs more investigation state/awaiting-triage Any issues or pull requests which haven't yet been triaged type/bug Any issue which is a bug or PR which fixes a bug labels Mar 14, 2024
@agjohnson
Copy link
Author

Also, and this is a complete aside, but colored inverted segment shows this same behavior. It is a bit more of a bug there due to the contrast ratio and more saturated background though:

image

But I'll focus this issue on message first however, segment feels harder.

@lubber-de
Copy link
Member

Try to make use of the text element which solves your issue. See https://jsfiddle.net/wLvnfzyj/

<a href="#"><span class="ui purple text">I am a purple link</span></a>

image

@lubber-de lubber-de added type/usage Any support issues asking for help and removed type/bug Any issue which is a bug or PR which fixes a bug state/awaiting-triage Any issues or pull requests which haven't yet been triaged state/awaiting-investigation Anything which needs more investigation labels Mar 15, 2024
@lubber-de
Copy link
Member

lubber-de commented Mar 15, 2024

Otherwise, there are global site definitions how a link should be displayed.

a {
color: @linkColor;
text-decoration: @linkUnderline;
}
a:hover {
color: @linkHoverColor;
text-decoration: @linkHoverUnderline;
}

You can adjust that using a custom theme and override those variables

@linkColor: #4183c4;
@linkUnderline: none;
@linkHoverColor: if(iscolor(@linkColor), darken(saturate(@linkColor, 20), 15, relative), @linkColor);
@linkHoverUnderline: @linkUnderline;

@agjohnson
Copy link
Author

I'm aware of the temporary fix with .ui.text classes. I'm instead resolving this on our project with overrides for each colored segment and the link pseudoclasses. To clarify, for our own purposes, I don't need any guidance here.

Instead, I was wondering if this should be changed (or a variant/feature added) for this directly in FUI.

This feels like FUI should do this without additional elements or style adjustments needed. Other components/modules decide at least text color for inherited elements, without explicit utility classes/etc -- colored labels comes to mind.

For some comparison, Bootstrap uses explicit utility classes for this as well (as I'd expect for Bootstrap though):

https://getbootstrap.com/docs/4.0/components/alerts/#link-color

I get backwards compatibility concerns here too, so maybe with no strong prior art in FUI for link colors this doesn't make for a helpful change. Would an extra variant perhaps be a path to proceed?

I'm looking to see if this is something I can help add to FUI. I'm happy fixing these issues on my own project -- it's all straight forward work there.

@lubber-de
Copy link
Member

Ok, understood 🙂

To make such feature consistent, this would need to support lots of components which provide color background variants as all of those could technically contain a link:
So far that's: nag, message, segment, table cells/rows, (basic) card, menu, popup, progress, toast

However, as some elements already support .link (label, menu item, icon, step, card) this should be carefully implemented as

.ui.{color}.message a.link { color: #xxxxxx;}

would also target a possible .link.label or .link.card inside a message where the color change is not desired as your intention is to support typical colored text links only as far as i understood.
But doing stuff like

.ui.{color}.message a.link:not(.card):not(.label):not(.menu):not.....

will blew up the selector

and even using direct child selectors like

.ui.{color}.message > a.link { color: #xxxxxx;}

does most probably not cover all possible use cases (for example a link inside a message header)

Each link would also need to support :active :hover :focus and :visited

A generic .ui.{color} would also match elements where the color class does not affect the background (like .ui.segment:not(.inverted).
Also inverted {color} does not always affect the background (such as card)

Yes, the whole color usage needs an overall refactoring for consistent/generic color/inverted logic in all FUI components (targeting for 3.0)

In conclusion: I somehow feel, we should not add a colored link class for each existing element into the FUI code as the text workaround as a generic solution works out "nice/safe enough" and i fear lots of used cases to be covered. But perhaps i am left alone with that opinion.

However, i am curious and very interested in how it worked out for your project: How did you enhance the fui logic? What's your final selector syntax? Maybe my thoughts were unnecessary complicated.

@lubber-de lubber-de added the type/discussion Anything which is up for discussion label Mar 15, 2024
@agjohnson
Copy link
Author

Thanks, that's a really helpful overview! Seems there is still plenty of complexity in even link colors 🙂

Yes, the whole color usage needs an overall refactoring for consistent/generic color/inverted logic in all FUI components (targeting for 3.0)

Aah yes, I knew this was a focus on 3.0 too, but I didn't connect these two together. I can see how that would affect what I'm suggesting here pretty quickly.

However, i am curious and very interested in how it worked out for your project

It's been pretty straight forward. I tend to use new, local variants (.ui.inverted.red.monotone.segment) so these changes aren't destructive to the default theme, and end up writing rather explicit selectors, because it doesn't need to be reusable. So, I'm also dodging much of the complexity at the FUI definition level with what I'm doing.

In conclusion: I somehow feel, we should not add a colored link class for each existing element

I probably agree, it's sounding that way. I wasn't thinking of all the other elements where this could be an issue still. I think I could pretty quickly get to a solution just for message or segment, but a generic solution sounds quickly overwhelming.

I appreciate the thoughts here though! If I come up with something reusable here, I'll come back to this, but maybe it's worth keeping in mind for a 3.0 or color refactor.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/discussion Anything which is up for discussion type/usage Any support issues asking for help
Projects
None yet
Development

No branches or pull requests

2 participants