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

Setting list color to silver sets it to None instead and other related color issues #5382

Open
C0rn3j opened this issue Apr 18, 2024 · 3 comments

Comments

@C0rn3j
Copy link
Contributor

C0rn3j commented Apr 18, 2024

Issue

Server Setup Information

  • Did you test in newest Wekan?: Y - 7.43.0
  • Deployment Method: Docker
  • Http frontend: nginx
  • Node.js Version: latest Docker
  • MongoDB Version: latest Docker
  • web browser version: latest Chromium

Problem description

Setting card color to silver works fine.

Setting list color to silver sets it to None.

Other colors are fine.

@C0rn3j
Copy link
Contributor Author

C0rn3j commented May 4, 2024

@xet7
I found the culprit:

wekan/models/lists.js

Lines 341 to 343 in 902f86d

if (newColor === 'silver') {
newColor = null;
}

wekan/models/swimlanes.js

Lines 296 to 298 in 902f86d

if (newColor === 'silver') {
newColor = null;
}

Current status of colors -

Green:
image

Silver - hardcoded HTML - CSS makes the color unset, looks the same when color is set to #e4e4e4:
image

Null - identical to Silver, at least in this view:
image

White (broken):
image

Null UI when opening colors defaulting to white:

image

This is however broken, when opening the UI, white looks selected, but in reality isn't.
One has to click on it to be ACTUALLY selected, and then it applies, but it just looks broken and moves the text element as opposed to every other color.

Additionally, the CSS for white is broken, resulting in broken rendering:

image

Fixing the CSS by commenting out the extra border line looks like this:

image

# client/components/lists/lists.css
.list-header-white {
  border-bottom: 6px solid #fff;
#  border: 1px solid #eee;
}
.list-header-green {
  border-bottom: 6px solid #3cb500;
}
.list-header-yellow {
  border-bottom: 6px solid #fad900;
}

So there are actually multiple issues with List colors.

  1. Broken CSS for white List color - Fixed in Fix white List color and define Silver in CSS instead of leaving it unset #5400
  2. List refuses to be set to silver because it is the default color, which may be a fine idea, but this is not really obvious and the UI makes it even more confusing by both allowing to SET silver and UNSET color, both of which null out the color value - Fixed in Allow silver color to be set in List and Swimlane #5401
  3. Because silver is null, default selected color when null is defined will be position 0, which is white, not silver
  4. Color selector does not actually have the element selected, so using Save upon just opening the dialogue is broken (made apparent by the 3rd issue)
  5. Because SETting silver actually unsets the color value, it confuses the poor programmer (me) who is trying to set Card colors to the same color as the List and not seeing it in the List data
  6. Since the CSS is unset in the case of null, some designs are breaking, for example:
    image
    Which is very apparent when it's the last List in the list

I suggest to actually set color to silver if user/API forces it via Save or Edit, and just have null be the default value as it is right now, which currently points to silver, but actually set it in CSS.
Perhaps someone would want to theme WeKan in the future, and not being able to change the now-different default to silver would impede them.
Dark Reader also does not seem to be enjoying the unset value:

Silver:

image

In which case it would be nice to actually set the color here too to #e4e4e4 (because it's not currently valid CSS) instead of relying on the background: CSS file itself fixed in #5400 but is not being applied yet

.list-header-silver {
  border-bottom: 6px solid unset;
}

@xet7
Copy link
Member

xet7 commented May 4, 2024

Fixed at #5400

@xet7 xet7 closed this as completed May 4, 2024
@C0rn3j
Copy link
Contributor Author

C0rn3j commented May 4, 2024

That unfortunately only fixed one of the six issues, please re-open.

@xet7 xet7 reopened this May 4, 2024
@C0rn3j C0rn3j changed the title Setting list color to silver sets it to None instead Setting list color to silver sets it to None instead and other related color issues May 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants