search

processing / p5.js-web-editor

The p5.js Editor is a website for creating p5.js sketches, with a focus on making coding accessible and inclusive for artists, designers, educators, beginners, and anyone else! You can create, share, or remix p5.js sketches without needing to download or configure anything.
https://editor.p5js.org
GNU Lesser General Public License v2.1
1.37k stars 1.32k forks source link

Discussion: `TextLink` component #2947

Open lindapaiste opened 8 months ago

lindapaiste commented 8 months ago

Tasks

Increasing Access

We want to keep the codebase clean and keep the design consistent.

Feature enhancement details

We have had many issue recently relating to the visibility of text links throughout the app. I would like to centralize the discussion here.

The recurring problem is that it's not clear to the user that a certain text is a clickable link. We can improve this by changing the default style of the link as well as adding hover styles to make it look more interactive.

I don't want us to add lots of specific CSS to specific component because this is difficult to maintain. I think this is a good opportunity for us to expand our library of reusable core UI components with a TextLink. We might not want to have the exact same styles everywhere, so we'll want to have some props which can control styles.

The goal of this issue is to discuss our API for the TextLink component and finalize what props we want.

Depending on the API that we choose, the code might look like any of these:

<TextLink href={url} underline="hover" color="always">anchor</TextLink>
<TextLink href={url} underline="hover" color="primary">anchor</TextLink>
<TextLink href={url} primary bold>anchor</TextLink>

^ This last one uses boolean props and is concise but probably more confusing.

Ideas for props:

Specific related issues:

References / how others handle this:

PiyushThapaa commented 8 months ago

There can be multiple combinations of this possible but, I think what's more appealing is that making all the links bold or simply an underline and adding theme color on hover.

So the code will look like either : <TextLink href={url} underline="always" color="hover">anchor</TextLink> OR <TextLink href={url} fontWeight="bold" color="hover">anchor</TextLink> (Correct me if I am wrong)

But if Community don't want this idea...

I think the code should be : <TextLink href={url} underline="hover" color="primary">anchor</TextLink>

What do you think? @lindapaiste @raclim and others

Mubashirshariq commented 8 months ago

<TextLink href={url} underline="hover" color="always">anchor</TextLink> @lindapaiste @raclim i think the above one prioritizes user accessibility and visibility. The dynamic underline on hover enhances link recognition, while persistent coloring ensures consistent visibility, contributing to an intuitive and user-friendly design.

Keshav-0907 commented 8 months ago

Yes indeed, this is a great idea! Introducing a centralized TextLink component will not only enhance the visibility of clickable links but also contribute to a cleaner and more maintainable codebase.

I'm want to actively contribute on implementing this TextLink component. If you have any further suggestions or considerations, feel free to share them!

rahulrana701 commented 8 months ago

Hi @lindapaiste, I believe the best approach for this is to utilize the Recoil library. With this library, we can define all the props in a separate file along with the functionalities they will perform. Afterwards, we can simply use the functions provided by this library and utilize whichever prop we need throughout the entire application.

The main advantage of this library is that it will save us from prop drilling and the re-renders that often occur. It achieves this by allowing any component to subscribe only to the specific props they actually need without worrying about parent and child components , and without necessitating the passing of props from parent to child.

This not only reduces unnecessary re-renders but also makes our code much cleaner and easier to manage If you're interested in pursuing this solution,

I'd be happy to work on this issue. You could assign it to me, and I can get started.

lindapaiste commented 8 months ago

Hi @lindapaiste, I believe the best approach for this is to utilize the Recoil library. With this library, we can define all the props in a separate file along with the functionalities they will perform. Afterwards, we can simply use the functions provided by this library and utilize whichever prop we need throughout the entire application.

The main advantage of this library is that it will save us from prop drilling and the re-renders that often occur. It achieves this by allowing any component to subscribe only to the specific props they actually need without worrying about parent and child components , and without necessitating the passing of props from parent to child.

This not only reduces unnecessary re-renders but also makes our code much cleaner and easier to manage If you're interested in pursuing this solution,

I'd be happy to work on this issue. You could assign it to me, and I can get started.

I’m familiar with Recoil and I think it’s pretty cool. We’re deeply entwined with using Redux as our state management system so I don’t have any plans to change that.

Also I don’t see how Recoil relates to this issue at all. I think you might be misunderstanding the discussion.

The idea is that there are certain props of a TextLink which might be different in various places. For example we might want underlines for links on the Sign In page, which appear in the middle of a sentence and need more distinction, but not want underlines on the lists of links in the About popup. So we will create a TextLink component that has a prop which tells us whether or not to show an underline on that particular link.

rahulrana701 commented 8 months ago

Hi @lindapaiste, I believe the best approach for this is to utilize the Recoil library. With this library, we can define all the props in a separate file along with the functionalities they will perform. Afterwards, we can simply use the functions provided by this library and utilize whichever prop we need throughout the entire application. The main advantage of this library is that it will save us from prop drilling and the re-renders that often occur. It achieves this by allowing any component to subscribe only to the specific props they actually need without worrying about parent and child components , and without necessitating the passing of props from parent to child. This not only reduces unnecessary re-renders but also makes our code much cleaner and easier to manage If you're interested in pursuing this solution, I'd be happy to work on this issue. You could assign it to me, and I can get started.

I’m familiar with Recoil and I think it’s pretty cool. We’re deeply entwined with using Redux as our state management system so I don’t have any plans to change that.

Also I don’t see how Recoil relates to this issue at all. I think you might be misunderstanding the discussion.

The idea is that there are certain props of a TextLink which might be different in various places. For example we might want underlines for links on the Sign In page, which appear in the middle of a sentence and need more distinction, but not want underlines on the lists of links in the About popup. So we will create a TextLink component that has a prop which tells us whether or not to show an underline on that particular link.

Hi @lindapaiste my plan was not to change the state management library I understood the issue very well , my goal was to have recoil library only to control the css of these links and use them wherever we want instead of creating a component for these , but if you want a component for them I am up for that as well.

lindapaiste commented 8 months ago

Responding to https://github.com/processing/p5.js-web-editor/issues/2940#issuecomment-1910977034 - I want to centralize the discussion here.

@lindapaiste Hmm, I think the primary accent color might not work because it doesn't pass the color contrast criteria for accessibility.

Screenshot 2024-01-25 at 3 42 00 PM

I think we actually have a lot of elements within the site that definitely don't pass this 😅 particularly, the dark grey text on light grey backgrounds. I've been wondering in terms of appearance, if it might be better to reach out to a designer who can give a clear direction on how all these elements should look that also aligns with these standards?

I think it's important that we follow these best-practices. Honestly I love rules because it removes a lot of subjectivity and helps me make decisions. So I've been reading a bit.

The standard is that when a link is presented inline with other text it should have an underline, regardless of color. That will make it so that links are obvious and fix most of the current issues. The underline is not necessary when we have an entire element that's a link. It's also acceptable to use bold instead of underline but IMO underline is more easily understood as a link.

W3 says:

While some links may be visually evident from page design and context, such as navigational links, links within text are often visually understood only from their own display attributes. Removing the underline and leaving only the color difference for such links would be a failure because there would be no other visual indication (besides color) that it is a link.

The contrast ratio is close enough to passable that maybe we just need a slightly darker variation of the pink to use as a text link color for light mode? At least, it's very close to meeting the AA-level guideline. We have a ratio of 4.2 with #ED225D. It only needs to be 4.5 to pass AA but 7.0 to pass AAA. So AAA would require going with a significantly darker pink (around #AF0E3C) or introducing a secondary accent color. A slight tweak like #E7134F gets us up to 4.5. In dark mode we're at 5.0 so that's passable already.

raclim commented 8 months ago

@lindapaiste Thanks for looking into this!

I'm down with using the underline as well over bold. I think the editor has a lot more standalone links (like in the About Modal) than inline ones. Maybe for those types of links the underline could show up on hover?

I guess I was feeling a little hesitant about implementing color changes that might stray from p5.js' identity, but the 4.5 #E7134F tweak doesn't feel too different!

As an added note, I think looking at school/university websites would also be great for more references/examples because I think they've been required to meet these standards and are probably already implementing a lot of these (for example, NYU had an initiative to be AA compliant back in 2017).