Skip to content

Conversation

@andersonjeccel
Copy link
Contributor

@andersonjeccel andersonjeccel commented Apr 10, 2024

Q A
Bug fix? (use the a.b branch) [ ]
New feature/enhancement? (use the a.x branch) [ ]
Deprecations? [ ]
BC breaks? (use the c.x branch) [ ]
Automated tests included? [ ]
Related user documentation PR URL mautic/mautic-documentation#...
Related developer documentation PR URL mautic/developer-documentation#...
Issue(s) addressed Fixes #...

Description:

Edit

Since we didn't get to a conclusion and here [UI] Accessibility features in user profile I add the underlined links as a feature, I used this PR to just assign the links and body text color to the right tokens.

Contrast Ratio goes from 9.62:1 to 18.1:1 for text color. Both passed AAA WCAG guidelines, but the new darker color feels like a good improvement for the eyes

Links remained the same for now...

Before:

image

image

After:

image

image

Steps to test this PR:

  1. Open this PR on Gitpod or pull down for testing locally (see docs on testing PRs here)
  2. Click on the top navbar links
  3. Click on the menu items
  4. Click on links you find
  5. Look at the general text color

@andersonjeccel andersonjeccel self-assigned this Apr 10, 2024
@andersonjeccel andersonjeccel requested review from a team, Mike-Dropsolid and code5rick April 10, 2024 19:11
@andersonjeccel andersonjeccel added T1 Low difficulty to fix (issue) or test (PR) user-experience Anything related to related to workflows, feedback, and navigation enhancement Any improvement to an existing feature or functionality accessibility Any issues relating to accessibility labels Apr 10, 2024
Copy link
Contributor

@LordRembo LordRembo left a comment

Choose a reason for hiding this comment

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

I have an alternative suggestion: underline the links instead of making them bold.
The most easily reconizable characteristic of an link, is an underline.
Making it bold introduce more ambiguity, because it will just look the same as the table headings.

@LordRembo
Copy link
Contributor

From an Accessibility standpoint, links should not be visual styles similar to other elements in the content/text (like bold or highlight) but visually distinct. Eg. they can be bold or highlighted if you absolutely want them to be, but then they should be designed to look like they are clickable (eg. look like a button or have an icon or add an underline in addition to the bold/highlighted style)

@Mike-Dropsolid
Copy link

I have an alternative suggestion: underline the links instead of making them bold. The most easily reconizable characteristic of an link, is an underline. Making it bold introduce more ambiguity, because it will just look the same as the table headings.

I know that's the theory but it does feel... weird/uncommon to underline it in this particular use case. I checked a few tools (WordPress, Accountable, Combell, ...) and they all put it in bold with a clear hover. Theory vs habit?

@andersonjeccel
Copy link
Contributor Author

@LordRembo @Mike-Dropsolid

The ideal scenario here would be to keep a normal font weight but apply a saturated color. For the Mautic brand, it could be the yellow or green (everyone would probably find it weird since these colors are rarely used on UI). Is it there a possibility for us to consider? Could I make all links yellow or green?

An underline is commonly used for accessibility improvement as a feature which you can enable/disable but it’s not exactly often used on the “normal theme”/for all users… We would also have the same issue since columns header are also a link

The point about theory vs habit is important: things become recognizable through repetition. If we keep everything consistent, probably it would be all good to proceed this way

@LordRembo
Copy link
Contributor

If we can get another review that agrees with just making it bold, I'm fine with it. In the end, consistency and habit are the best indicators if something will work or not. No need to change the colors or styles to something that is not used anywhere.

@code5rick
Copy link
Contributor

code5rick commented Apr 12, 2024

I like the proposed idea. My only suggestion would be to consider using a slightly lighter font weight for the link probably. Additionally, I would recommend making the links in the 'thead' regular font weight instead of bold, to distinguish them from the links in the list. Other than that, it looks good! 👌

@andersonjeccel
Copy link
Contributor Author

@code5rick We would fall into the issue mentioned on the feedback, now for thead links, if they become not-bold

@RCheesley Any UX idea here?

@RCheesley
Copy link
Member

I'm afraid I'm from the old school ... underlines signify to me that something's a link! 🙈

I like how Joomla does it with their accessibility plugin which ships with core and can be turned on, it gives the user the power to change links to be underlined and a bunch more stuff based on their own needs. You can see it in action on my site here. Could be something to consider in the future, maybe.

From their article, it states:

WCAG does not require the links to be underlined (they must be highlighted). But underlined links can improve accessibility for visually impaired people or for people who have difficulty concentrating.

I think making them bold does help if we don't want to underline. I don't think yellow/green would help as those colours don't go well on a white background, but maybe a hover of those colours (probs the Mautic yellow) might assist with signifying a clickable link, if we really want to avoid underlining.

@andersonjeccel
Copy link
Contributor Author

@code5rick
Our font weight options are:

300 (light)
400 (normal, currently used)
600 (semibold)
700 (bold, on the screenshot)

I can set 600 to be a bit thicker, but should it be applied to all links on the interface or only to the ones on the second column of all tables?

@andersonjeccel
Copy link
Contributor Author

@RCheesley It would be good if we had a feature like this (lots of enable/disable for each accessibility item)

If hard to implement, a high contrast theme with standard accessibility changes would also work fine (kind of a LESS file where accessibility features overwrite the normal theme rules)

Regarding the link on tables, this one is hard. Could you select 1 way to proceed, then we start a slack discussion to decide if we implement it or not?

The options:

  1. Bold (increase visibility, subtle, not accessible)
  2. Underline (increase visibility, commonly recognized but not subtle at all, accessible)

(an additional comment: is this comparison fair? Is it right to compare accessible vs non-accessible? If not, would bold be the only option? Should we give up on this change for now?)

@RCheesley
Copy link
Member

So, these are the options I think we're considering here:

screenshot-mautic ddev site-2024 04 19-14_58_25

Personally, I don't think that making it bold gives enough of direction to the user that it's a click-able link. I think that regular underline (without bold) would be the best option for making it clear it's a link, but it doesn't look so 'tidy'.

So I would indeed put this out for discussion in the community and get wider viewpoints.

@andersonjeccel andersonjeccel marked this pull request as draft April 19, 2024 16:46
@andersonjeccel andersonjeccel changed the title [UI] Assign links and body text color to tokens [UI] Assign links and body text color + focus state to tokens Jul 12, 2024
@escopecz
Copy link
Member

@andersonjeccel there's a conflict now.

@escopecz escopecz added the has-conflicts Pull requests that cannot be merged until conflicts have been resolved label Jul 15, 2024
…e-that-you-dont-know-are-links-is-one-example
@andersonjeccel
Copy link
Contributor Author

@escopecz Conflicts solved!

@andersonjeccel andersonjeccel removed the has-conflicts Pull requests that cannot be merged until conflicts have been resolved label Jul 15, 2024
…e-that-you-dont-know-are-links-is-one-example
@escopecz escopecz mentioned this pull request Jul 16, 2024
…e-that-you-dont-know-are-links-is-one-example
@escopecz
Copy link
Member

@RCheesley @kuzmany @LordRembo could you please give this PR another try? Anderson made more changes according to your feedback. I don't want to get this PR merged until the majority agrees. At the same time, this PR is blocking a bunch of others so we are stuck without your feedback.

Copy link
Member

@RCheesley RCheesley left a comment

Choose a reason for hiding this comment

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

Thanks for the nudge @escopecz - @andersonjeccel I think there's a discrepancy with the contact list, because the blue rectangle doesn't show when you click: https://app.screencastify.com/v3/watch/YWqdjuaHnz4krL9k2QOl.

Is this what I'm meant to be testing? I wasn't fully sure from the description and couldn't see massive differences between 5.x and this PR.

@andersonjeccel
Copy link
Contributor Author

@RCheesley Thanks for taking time to review!

Hm, we have a difference between the way different page lists were created over the time
Specifically this one doesn't show because someone placed the names inside a <div> which is inside the actual link <a> (😭) so you're not technically clicking the link

I can take a note to fix this with other PR specifically for minor fixes like HTML structural changes, but it would be great to move this PR here forward to avoid text contrast issues with the light gray tile components 🙏

@RCheesley
Copy link
Member

OK @andersonjeccel perhaps you could be really specific then on what we should be reviewing? As I wasn't really sure from the original description / conversations in this PR what was and wasn't to be reviewed and what was/wasn't being changed specifically in this PR.

@andersonjeccel
Copy link
Contributor Author

andersonjeccel commented Jul 18, 2024

@RCheesley You should review:

  • Contrast (see if the text color feels better to your eyes)

@andersonjeccel andersonjeccel requested a review from RCheesley July 18, 2024 13:26
…e-that-you-dont-know-are-links-is-one-example
…nks-in-small-grey-typeface-that-you-dont-know-are-links-is-one-example
@andersonjeccel andersonjeccel mentioned this pull request Jul 31, 2024
…nks-in-small-grey-typeface-that-you-dont-know-are-links-is-one-example
@andersonjeccel andersonjeccel changed the title [UI] Assign links and body text color + focus state to tokens [UI] Assign links and body text color Aug 1, 2024
@andersonjeccel andersonjeccel changed the title [UI] Assign links and body text color [UI] Assign links, body and control-label text color to tokens Aug 1, 2024
@andersonjeccel
Copy link
Contributor Author

I reverted the outline to ensure this PR will move forward, it's blocking great improvements like

Copy link
Contributor

@LordRembo LordRembo left a comment

Choose a reason for hiding this comment

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

Right, yeah, I'm fine with this. Sorry, thought I had already re-approved it.

@andersonjeccel andersonjeccel added ready-to-commit PR's with 2 successful tests, 1 approval, automated tests and docs and is ready to be merged user-testing-passed PRs which have been successfully tested by the required number of people. and removed pending-test-confirmation PR's that require one test before they can be merged labels Aug 2, 2024
@escopecz escopecz merged commit 4de07e2 into mautic:5.x Aug 5, 2024
@andersonjeccel andersonjeccel deleted the ux-More-intuitive-links-in-small-grey-typeface-that-you-dont-know-are-links-is-one-example branch August 5, 2024 11:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

code-review-passed PRs which have passed code review enhancement Any improvement to an existing feature or functionality ready-to-commit PR's with 2 successful tests, 1 approval, automated tests and docs and is ready to be merged T1 Low difficulty to fix (issue) or test (PR) user-interface Anything related to appearance, layout, and interactivity user-testing-passed PRs which have been successfully tested by the required number of people.

Projects

Archived in project
Status: Done

Development

Successfully merging this pull request may close these issues.

7 participants