TiddlyWiki / TiddlyWikiClassic

TiddlyWiki Classic (not to be confused with TiddlyWiki5: https://github.com/Jermolene/TiddlyWiki5)
https://classic.tiddlywiki.com/
492 stars 114 forks source link

css: fix missing color palette reference #298

Closed PengjuYan closed 7 months ago

PengjuYan commented 7 months ago

Some color in ColorPalette do not appear in some css fragments where they should do. It doesn't have any consequence if you use the original ColorPalette. However, if you try to make your own ColorPalette, e.g., your own dark mode (other than Yakov Litvin's DarkModePlugin), you'll find some colors don't change as you expected.

  1. Here's a snapshot of what's my own dark mode color palette will exhibit in view mode, which is as expected: 20240125-114427
  2. Here in edit mode, which is not as expected: 20240125-114441
  3. Here it appears as what we want after the fix: 20240125-114454
YakovL commented 7 months ago

Thanks for the PR! I've changed the base to dev, please always branch from and do PRs to dev, not master (the latter is used for the released versions). I'm not sure yet what I can do to avoid this for other contributors in the future; if you have some ideas, please share them.

I second this suggestion. A while ago Eric has argued that this changes the behavior for some older themes, but I believe it changes it well enough (if background/foreground combination is not very readable, than it [i.e. custom ColorPalette] should be changed, but that's not the reason to keep different colors for inputs). So once you update the styles according to what I've suggested, we can definitely get this into 2.10.1!

YakovL commented 7 months ago

And let me highlight my appreciation of this contribution: I hope the community will become more active about the core and the docs (and ultimately about inviting new users), so I'm very glad when somebody suggests improvements

PengjuYan commented 7 months ago

Thank you for the warm comments!

Forgive my English, but I'm afraid I don't get it (or, the whole paragraph) (English is not my mother tongue. :-D):

update the styles according to what I've suggested

Do I need to do any specific response? I guess you already changed the merge base to dev. Or, did you suggest a better way around instead of changing the two .tid files directly?

YakovL commented 7 months ago

No problem, I mean this bit:

Please move these styles to StyleSheetColors (since there's no .txtOptionInput selector yet, add it, for instance, after the h2,h3 selector with an empty line separator): they define colors!

I.e. this PR should only have changes in StyleSheetColors. (if it's still unclear, I'll create a commit for you)

PengjuYan commented 7 months ago

@YakovL I think I got it finally. And, I got a deeper understanding of the organization of the .tid files (you can imagine that it is not so straightforward for one who has no experience in JavaScript/css like me).

Check out the commit to see if it is the right way to do it. Thanks again.

PS: I'm also doing something inspired by your DarkModePlugin.

YakovL commented 7 months ago

Great, exactly what's needed. Merged, thanks again!

By the way, your native language is Chinese, isn't it? Have you seen the updated Translations repo? Both the Traditional and Simplified Chinese translations are quite outdated (they are for TW 2.4.1), so you may be interested in making them up-to-date; I've prepared a history of core changes, which can be helpful, but it covers versions 2.6.0-2.10.0, so it is a bit incomplete for this. Still, there's the English template, which can be used to look up the current texts. If you get interested, let me know, I can help with this.

PengjuYan commented 7 months ago

PS: I'm also doing something inspired by your DarkModePlugin.

Hi @YakovL, as I'm writing another plugin inspired by your work DarkModePlugin, I'm wondering how to inherit your LICENCE. Do you think what here says is a good choice?

Original work Copyright (c) 2012 [Acme Corp] Modified work Copyright 2012 Steve Bennett

PengjuYan commented 7 months ago

Great, exactly what's needed. Merged, thanks again!

By the way, your native language is Chinese, isn't it? Have you seen the updated Translations repo? Both the Traditional and Simplified Chinese translations are quite outdated (they are for TW 2.4.1), so you may be interested in making them up-to-date; I've prepared a history of core changes, which can be helpful, but it covers versions 2.6.0-2.10.0, so it is a bit incomplete for this. Still, there's the English template, which can be used to look up the current texts. If you get interested, let me know, I can help with this.

Thank you!

And you're right, my first language is Chinese. I just took a glimpse on the (simplified Chinese) translation and found that the wording was not written by someone from the mainland China. I'd like to contribute, but the timing doesn't seem right. I'll let you know when I'm free.

YakovL commented 7 months ago

No problem, take your time!

As for the license question, it depends on what exactly you'd like to create. I'm definitely not a legal expert, neither I'd bother much about licenses (the MIT one basically sais that you can reuse the code any way you like). If you are creating a fork of DMP with some tweaks, then it's a common approach to just add another slice that makes it clear that it's a fork, for instance:

|Author|[Pengju Yan| |Original Author|Yakov Litvin| |Forked from|https://github.com/YakovL/TiddlyWiki_DarkModePlugin/|

this should work fine as it points that this is a fork, where to find it and its license, and who was behind the previous code.

If you're creating a plugin that just uses some bits of DMP, I think it's enough to add a slice like

|Acknowledgements|Parts of the code were inspired by https://github.com/YakovL/TiddlyWiki_DarkModePlugin/ (by Yakov Litvin)|

Again, "this is not a legal advice" (I'd love if somebody consults the community how to do things correctly), but nobody will sue you for reusing Open Source anyway, this more of a common sense and politeness advice.

PengjuYan commented 7 months ago

Thank you. I've revised the LICENCE a little bit by adding my name on it. :-)

I also added an Acknowledgements row in the description of the plugin.

You can check out my SwitchPalettePlugin. This is my first JavaScript project. Looking forward to any advice.