Expression-Search-NG icon indicating copy to clipboard operation
Expression-Search-NG copied to clipboard

Full dark theme support implementation (ready for PR)

Open oleole39 opened this issue 11 months ago • 1 comments

Hello,

Context

That issue refers to following @opto's request

help would be appreciated in other stuff. For example: a) the notreply button is needed in a version for dark mode: skin/noreply1black.svg But I have no idea at the moment whether these icons are still loaded as svg image in 115 or in a class. I think in a first trial the loading as image was not displayed in 115. b) there are some proposals for colors in the popup in dark mode. (here on github). We need a second css file with those replacing the standard, and in loading the addon need to detect whether there is dark mode or not (I have that in some other addon - maybe Nostalgy++, or even here?), and add that code and load the other stylesheet under that condition.

Some documentation

Quickfilters icons are still loaded in TB115 as transparent SVG files which are colorized via CSS.

  1. SVG
    1. Icons are associated to HTML button items in quickFilterBar.css through variables assigned to CSS background-image property.
    2. Those variables are defined in icon.css as links to SVG files.
  2. Icon colors
    1. Included SVG files are fully transparent - their fill color is managed by a CSS file specific to a given context. For quickfilters icons, this CSS file is widgets.css
      1. CSS property background-color defines the color of the button's background color.
      2. CSS property color defines the button's font color as well as the SVG ic as a starton's border color.
      3. CSS property fill defines the inner SVG icon's color. It is set as a gradient with 20% transparency of the color defined at property color.
    2. Color variables are defined in layout.css and are used under prefers-color-scheme CSS media queries to manage dark, light and high contrast themes.

Change proposal

This branch aims at implementing full dark theme support for ESNG:

  1. Refining overlay dark theme structure
    1. 'notreply'/unreplied icon issue. I assume opto's point refers to that issue. At the light of above documentation, modifying the SVG icon to make it transparent using context-fill or context-stroke values as colors properties (instead of harcoded values for 'black') seems enough to fix the issue.
    2. Deprecated JS dark theme detection mechanism (in /api/ExpressionSearch/implementation.js) in favor of a simple CSS media query prefers-color-scheme: dark and color variables in /skin/overlay.css. Thus removing overlaydark.css which is not used anymore.
    3. Make the tooltip element's background-color property customizable via the CSS stylesheet applying .forceInfo class to it in any case (and not just in a specific case anymore) in /modules/ExpressionSearchChrome.jsm.
  2. Implementing HTML popups' and pages' dark theme structure
    1. Implemented via a unique prefers-color-scheme: dark CSS media query. Basically combined all related CSS stylesheets and inline CSS code into a unique CSS stylesheet /html/popups.css. To make this last, all new styles should ideally be added to the stylesheet (i.e. avoid the use what might be the default output of some WYSIWYG editor adding inline CSS everywhere).
  3. Dark theme design
    1. All HTML & CSS code has been reworked having in mind to mimic the previously existing design of the add-on, with a few exceptions:
      1. Introduced color groups for the operators of the search popup.
      2. "Arbitrarily" defined some dark theme colors for the popups & pages (as there was no previously existing dark theme for them).
    2. Principles of the new theme strucure
      1. The new theme structure is inspired of native TB theme implementation.
      2. It makes use of 2 types of CSS variables defined in each stylesheets (overlay.css for overlay styling and popups.css for popups styling). It means that to change the theme's palette, one only has to tweak those variables in the CSS files:
        1. color variables, to which are assigned hex, RGBA or HTML color codes and which are conventionnally named in the following format --ESNG-color-blue-2 :
          • ESNG to make sure to have an add-on specific name
          • color to refer to color variables
          • blue to give a broad idea of the color family
          • 2 referring to the shade (0 being the lightest of the family defined in the stylesheet)
        2. template variables, to which are assigned one color variable for each theme variant (thus TB will automatically select one color or its alternative depending the detected theme's preference), and which are conventionnally named in the following format ESNG-popups-background-0 :
          • ESNG to make sure to have an add-on specific name
          • popups to refer to the popups CSS stylesheet
          • background to refer to the CSS property it is assigned to in the stylesheet (here background or background-color)
          • 0 as a differentiator (no particular meaning)
        3. It works and to my mind it answers the current needs, although there is still room for future improvement (if someone is willing to work on this).
          1. Comment template variables to make it easier to understand their impact (thus saving future developpers from analyzing the full CSS file).
          2. Harmonize overlay.css dark theme design with popups.css dark theme deisgn. That would mostly make sense in the frame of a general redesign of the add-on's UI.
          3. Use TB internal color variables instead of color codes. Current design proposal hardcodes color codes. but for a seemless integration with all of user's specific themes, it could use the standard TB template variables, thus automically using the user's theme's code. That would take some research to find all adequate TB variables, and probably to redesign part of the addon's HMTL to reduce variations from one file to another. I coudn't find documentation for native color template variables, but they can be searched forvia the Dev Tools, or in TB's source code (there for instance).

Note: the branch mentionned also includes in its commit history the fix described in #66, even though the code it modifies is eventually commented out in a later commit relative to the deprecation of JS dark theme detection.

Remaining questions (testing required)

  1. I tested successfully the branch's version on TB 112.13.0 on Linux MInt 21 (Ubuntu 22.04 LTS-based), but it requires to be tested more extensively:

    1. It is probably compatible with lower versions of TB, at least until a certain point that I did not determine (I guess the main question is here in which TB version the support for dark theme media query been introduced ?)
    2. it remains to be tested on other OSes.
    3. Here is a zip of the branch version (its file extension has to be renamed in .xpi and it can then be drag'n'dropped to TB add-on manager tab for installation) if anyone feels like testing and reporting.
  2. On TB 115.0.1 on the same OS, dark theme works for pop-ups' and pages'. Other added features could nevertheless not be tested due to current partial ESNG incompatibility already mentionned by opto.

    • the unreplied filter button in the toolbar is not loaded at all (so couldn't check for its dark theme support)
    • the tooltip is not loaded at all (so couldn't check for its dark theme support)

To avoid double work, @opto, would you already have adressed this partly? Seeing your intermediate version would prove useful here (ideally with tracked changes, hence the use of #77). I could make a PR of the branch to this repo as well if you would like to.

oleole39 avatar Jul 26 '23 22:07 oleole39

This proposal aims at fixing #40, #44, #55, and #74 (first point) - to be confirmed with further testing.

oleole39 avatar Jul 26 '23 22:07 oleole39