p5.js-web-editor icon indicating copy to clipboard operation
p5.js-web-editor copied to clipboard

Published 20 hours ago • verified publisher icon processing

Friendly error links have poor color contrast

Open stampyzfanz opened this issue 3 years ago • 1 comments

Details about the bug:

  • Web browser and version: Firefox 102.0.1
  • Operating System: Windows 10
  • Steps to reproduce this: https://editor.p5js.org/stampyzfanz/sketches/flp7cojOM image The problem is inline CSS that changes the colour of FES links without taking into account the theme that is used.

stampyzfanz avatar Jul 30 '22 03:07 stampyzfanz

Welcome! 👋 Thanks for opening your first issue here! And to ensure the community is able to respond to your issue, be sure to follow the issue template if you haven't already.

welcome[bot] avatar Jul 30 '22 03:07 welcome[bot]

Hi there, if someone isn't already working on this, I'd like to have a crack! This would also be my first open source contribution :).

Brady-Edwards avatar Oct 29 '22 03:10 Brady-Edwards

I am not aware of anyone working on it and feel free to let me know if there's anything I can do to help.

Thanks for your interest :)

stampyzfanz avatar Oct 30 '22 09:10 stampyzfanz

I've got a working fix, can a maintainer give me the green light to open a PR? Thanks.

Brady-Edwards avatar Nov 01 '22 12:11 Brady-Edwards

Hi @Brady-Edwards, super excited to see your work! :) Please feel free to go through with the PR, or let me know if there's anything else you need beforehand!

raclim avatar Nov 09 '22 20:11 raclim

Hello. is this still open?

devtarun avatar Jan 07 '23 11:01 devtarun

Hi there. If this haven't been resolved yet then I would love to do so. Also can somebody point me where can I get slack or discord link where there is more discussion on issues. Thank you

ayushbhaimehta avatar Jan 16 '23 15:01 ayushbhaimehta

Hi @devtarun and @ayushbhaimehta! Yes, this is still currently open, but I think @Brady-Edwards had some work done on this earlier. However, please feel free to add to this issue!

This is the invite URL for the p5.js discord. :)

raclim avatar Jan 18 '23 04:01 raclim

Hi there, I had completely forgotten about this (got carried away with life)! I had already pin-pointed the issue and a fix for it but never got around to issuing a pull request. However, I'm more than happy for someone else to issue the pull request for this instead :). Either way, I'll quickly detail what I found to cause the issue here:

In this snippet here, the color and fill lines are getting overridden with some external css (not sure what's the cause for the overwrite, but it doesn't seem to be from a css stylesheet in the repository). To prevent these from being overridden, the !important tag can be used, which seemed to fix the bug for me. I'm sure there's a better fix out there, but this is all I could find from the time I spent looking at this.

Brady-Edwards avatar Jan 18 '23 04:01 Brady-Edwards

@raclim I pretty much agree with brady. The CSS seemed to be overridden by 2-3 external modules of CSS (one is definitely from main but still can't figure out the other ones).

ayushbhaimehta avatar Jan 18 '23 10:01 ayushbhaimehta

I looked into this a bit so I want to get this written down for posterity. The TL;DR is that we are missing an important prop on a component from an external package.

Here's what I'm seeing in dev tools image

There is an appropriate link color #f0f0f0 that is set for all links when in .contrast, but that gets overwritten by another style which sets the link color for the console. That's the Emotion-generated CSS .css-kidsne a. The console element <div data-type="string" class="css-e393sm"> is a closer ancestor than the <body class="contrast"> so that style will get priority.

The console component comes from an external package 'console-feed', but we are passing in quite a lot of styling information to customize it.

What we are not doing, however, is passing a variant prop to the <ConsoleFeed/> component! The console-feed package supports light and dark modes, and the default styling will change based on which you select. You can see that in their source code here:

LOG_LINK_COLOR: isLight ? 'rgb(66, 66, 66)' : 'rgb(177, 177, 177)',

There are two ways to fix this:

  1. We can pass a variant="dark" prop to the <ConsoleFeed/> when in "dark" or "contrast" modes. This changes the default color from the unreadable dark gray rgb(66, 66, 66) to a light gray rgb(177, 177, 177)

Contrast: image Dark: image

We need to add one additional line to the code here

<ConsoleFeed
    variant={theme === "light" ? "light" : "dark"} // <---- this line
    styles={getConsoleFeedStyle(theme, fontSize)}
    logs={consoleEvents}
    key={`${theme}-${fontSize}`}
/>
  1. We can define our own LOG_LINK_COLOR as whatever precise color we want it to be. We would add an additional property to the CONSOLE_FEED_DARK_STYLES and CONSOLE_FEED_CONTRAST_STYLES objects.

I recommend #1 for simplicity.

lindapaiste avatar Feb 19 '23 00:02 lindapaiste