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

[BUG] [Regression] Tag pill style became ineffective in edit mode after 5.3.3 #8737

Open
CodaCodr opened this issue Nov 11, 2024 · 12 comments · May be fixed by #8739
Open

[BUG] [Regression] Tag pill style became ineffective in edit mode after 5.3.3 #8737

CodaCodr opened this issue Nov 11, 2024 · 12 comments · May be fixed by #8739

Comments

@CodaCodr
Copy link
Contributor

Describe the bug

Somewhere after 5.3.3, styles applied to tags stopped being applied to tags on tiddlers in edit mode.

5.3.5 (tw .com) (FAIL)

ee101b0083f2ee257b0a401ade0703f757d5875e

5.3.6pre (FAIL)

9dfae238c2398dee2550eda58d15cf3767f01d78

Expected behavior

https://tiddlywiki.com/archive/full/TiddlyWiki-5.3.3 (PASS)

f88a5f3dc8493b8f678978d1f30d14be71f01a89

To Reproduce

Add a color field with value: "my-tag".

The CSS style for testing:

span.tc-tag-label[style*='my-tag'] {
  border:1px solid #08c;
  background-color:white;
  color:#08f !important;
  font-size:1rem;
  font-weight:bold;
}

Screenshots

No response

TiddlyWiki Configuration

Win10, Firefox latest, Chromium latest.

Additional context

No response

@Jermolene
Copy link
Member

Thank you @CodaCodr

@pmario
Copy link
Member

pmario commented Nov 11, 2024

I can not replicate the problem.
Edit: I could as I did add the field color: my-tag -- Not happy with that one.

@pmario
Copy link
Member

pmario commented Nov 11, 2024

The CSS definition does not make any sense. It seems to be a complete misuse of the colour field and from my point of view is no regression

It only works for a tiddler named: my-tag as shown in the screenshot.

image

The right CSS for edit mode would be like this:

span.tc-tag-label[data-tag-title='my-tag'] {
  border:1px solid #08c;
  background-color:white;
  color:#08f !important;
  font-size:1rem;
  font-weight:bold;
}

I'll have to have a closer look how the CSS for edit mode should look like. More time needed.

Edit: Here is the CSS for edit mode. No color-field needed -- Only tags

[data-tag-title*='my-tag'] .tc-tag-label{
  border:1px solid #08c;
  background-color:white;
  color:#08f !important;
  font-size:1rem;
  font-weight:bold;
}

image

@CodaCodr
Copy link
Contributor Author

Thanks for taking a look, @pmario

But all you have done here is reveal the bug in a slightly different way, and, the problem is worse (the tag dropdown isn't working).

This, again, is the bug: Somewhere after 5.3.3, styles applied to tags stopped being applied to tags on tiddlers IN EDIT MODE.

Your code at tw .com

image

And please climb down talking about "complete misuse of the color field". It's CSS and CSS doesn't care. You're allowed to misspell color names. Treat it as a hack that reveals the problem. Fact: the demonstrated code works in 5.3.3 and earlier. It stopped working thereafter. That, sir, is a regression. Your code isn't working in any version.

Prediction: there is a commit somewhere that broke the formerly working code, where styles were applied to tiddlers in any mode.

@pmario
Copy link
Member

pmario commented Nov 11, 2024

You used the wrong CSS using TW prerelease

span.tc-tag-label[data-tag-title='my-tag'] {
  border:1px solid #08c;
  background-color:white;
  color:#08f !important;
  font-size:1rem;
  font-weight:bold;
}

image

@pmario
Copy link
Member

pmario commented Nov 11, 2024

The tag pill structure is slightly different between view-mode and edit-mode. Edit mode has the "close" button.

So you'll need both CSS definitions.

span.tc-tag-label[data-tag-title='my-tag'] {
  border:1px solid #08c;
  background-color:white;
  color:#08f !important;
  font-size:1rem;
  font-weight:bold;
}

[data-tag-title*='my-tag'] .tc-tag-label{
  border:1px solid #08c;
  background-color:white;
  color:#08f !important;
  font-size:1rem;
  font-weight:bold;
}

@pmario
Copy link
Member

pmario commented Nov 11, 2024

I think the main question is: What do you try to achieve?

  • As you know !important is toxic
  • Using hardcoded colour values in TW will cause problems with palette switching

So it would be important to describe, what you want to achieve and not how you want to achieve it. Then we can have a look, how we can solve the problem in a TW way. The initial CSS in the OP is very hacky because it uses a side-effect.

@CodaCodr
Copy link
Contributor Author

This is not a discussion about the merits of one chunk of CSS over another -- nor a topic about hackiness (already forewarned in the OP). Nor am I asking for you (or anyone) to help solve a problem. The report and its CSS REVEALS the bug/regression.

One look in the inspector of both releases reveals the outcome of the code change. The inline style is present in 5.3.3, missing from 5.3.5+. Everything else you ask for is present in the OP.

Then we can have a look, how we can solve the problem in a TW way.

First, I don't have a problem, I uncovered a regression, as previously documented.

To do this in a "TW way" is simple: Separate out the style to a Stylesheet tiddler, using your code (if it can be made to work) or mine (which is already working). However, I happen to prefer encapsulation for "little things". This little chunk of CSS sits nicely in the tiddler that describes the tag itself. Make it a stylesheet tiddler and bingo. This is an organic approach akin to storing the regular colour choice in the color field of the tiddler. If you can't see that, well, let's just move on. I'm completely okay with you not liking it.

@pmario
Copy link
Member

pmario commented Nov 11, 2024

Thanks for the info. Now I do understand the problem. I think I know where it comes from

@pmario
Copy link
Member

pmario commented Nov 11, 2024

@Jermolene @CodaCodr ... Going down the rabbit hole.

The $:/core/ui/EditTemplate/tags template uses TW v5.3.5 syntax.

To define styles we need to use html parameters like: style.background-color=<<backgroundColor>> .
The variable has the right value using the field color: my-tags as defined in the OP.

It seems browsers do sanitise those values if they are specific. my-tags is no valid color so it will be ignored. -> That causes the problem

Prove

If we use the field value color: red instead of color: my-tag the prerelease code works as seen in the screenshot below.

image

Using color: my-tag does not work because the browser ignores it.

image

What has been changed / Old code

The old v5.2.x code uses this construction to insert a style definition eg: <span style=<<tag-pill-styles>>

The macro is defined like this:

\define tag-pill-styles()
background-color:$(backgroundColor)$;
fill:$(foregroundColor)$;
color:$(foregroundColor)$;
\end

So it seems browsers do not validate "style-strings" if they are defined like this: <span style=<<tag-pill-styles>>>
They are restrictive for <span style.background-color=<<backgroundColor>> where they know it should be a colour.

Proposal

I do not have a proposal yet. -> More experiments needed.

@CodaCodr
Copy link
Contributor Author

Great detective work. Thank you.

If it is now decided (@Jermolene ?) to leave the current code in place (which, on the face of it, seems the right thing to do), I am happy with that choice. Legacy code sometimes needs to defer to more recent "better" code.

@pmario
Copy link
Member

pmario commented Nov 13, 2024

It can be resolved. PR is under way

@pmario pmario linked a pull request Nov 13, 2024 that will close this issue
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

Successfully merging a pull request may close this issue.

3 participants