The Svelte code should not have custom logic for the Gin theme

Created on 29 January 2025, 3 months ago

Problem/Motivation

There are a few places in the Svelte code where we are specifically checking for the Gin theme and doing logic around it. That's not really appropriate; the Svelte code can have opinions about dark mode, but shouldn't have any about Gin specifically.

Proposed resolution

On the PHP side, we can check for dark mode generically. We can check if Gin is installed, and if it's in dark mode, and then set drupalSettings.project_browser.darkMode to true. Then the Svelte code can handle dark mode as it sees fit, regardless of which theme sets it up.

🐛 Bug report
Status

Active

Version

2.0

Component

Code

Created by

🇺🇸United States phenaproxima Massachusetts

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @phenaproxima
  • 🇺🇸United States phenaproxima Massachusetts
  • 🇺🇸United States phenaproxima Massachusetts

    Started this but wasn't quite able to remove all mentions of Gin from the Svelte code, since we're also using it to conditionally apply classes to elements. Not entirely sure how to approach fixing that.

  • Pipeline finished with Canceled
    3 months ago
    Total: 251s
    #410349
  • Pipeline finished with Canceled
    3 months ago
    Total: 347s
    #410352
  • Pipeline finished with Canceled
    3 months ago
    Total: 85s
    #410362
  • Pipeline finished with Canceled
    3 months ago
    Total: 209s
    #410363
  • Pipeline finished with Canceled
    3 months ago
    Total: 100s
    #410365
  • Pipeline finished with Failed
    3 months ago
    Total: 358s
    #410366
  • First commit to issue fork.
  • Pipeline finished with Failed
    3 months ago
    Total: 477s
    #410988
  • Pipeline finished with Failed
    3 months ago
    Total: 417s
    #410995
  • Pipeline finished with Success
    2 months ago
    Total: 420s
    #413128
  • 🇮🇳India utkarsh_33

    This is ready for an initial round of review.

  • Pipeline finished with Failed
    2 months ago
    Total: 340s
    #413135
  • Pipeline finished with Success
    2 months ago
    Total: 357s
    #413137
  • 🇺🇸United States phenaproxima Massachusetts

    Unfortunately, I don't think switching from 'gin' in drupalSettings to GIN_ENABLED gains us anything -- the problem still exists, which is that we are conditionally applying classes based on the presence of the Gin theme. We need to not do that. The goal is for the Svelte code to not mention or care about Gin at all.

    Let's maybe talk through the CSS cascade here and see what we can do about it.

  • Pipeline finished with Failed
    2 months ago
    Total: 560s
    #413603
  • 🇺🇸United States chrisfromredfin Portland, Maine

    moving back to NW for now

  • Pipeline finished with Failed
    about 2 months ago
    Total: 1128s
    #433594
  • Pipeline finished with Failed
    about 2 months ago
    Total: 296s
    #433614
  • Pipeline finished with Failed
    about 2 months ago
    Total: 1213s
    #433620
  • Pipeline finished with Failed
    about 2 months ago
    Total: 1539s
    #433649
  • Pipeline finished with Failed
    about 2 months ago
    Total: 558s
    #433690
  • Pipeline finished with Success
    about 2 months ago
    Total: 1251s
    #433702
  • Pipeline finished with Success
    about 2 months ago
    Total: 1204s
    #433738
  • Pipeline finished with Success
    about 1 month ago
    Total: 605s
    #439350
  • Pipeline finished with Failed
    about 1 month ago
    Total: 567s
    #439374
  • Pipeline finished with Success
    about 1 month ago
    Total: 384s
    #439384
  • Pipeline finished with Success
    about 1 month ago
    Total: 1513s
    #439404
  • Pipeline finished with Success
    about 1 month ago
    Total: 1123s
    #439415
  • 🇺🇸United States phenaproxima Massachusetts

    Couple of questions but this seems like a big improvement to me.

  • Pipeline finished with Success
    about 1 month ago
    Total: 1207s
    #440369
  • 🇮🇳India utkarsh_33

    I have moved the css related to Gin into it's specific file.This is ready for review.

  • 🇺🇸United States phenaproxima Massachusetts

    Should probably get some manual testing but I think this looks like great and worthwhile cleanup!

  • Pipeline finished with Failed
    about 1 month ago
    Total: 413s
    #442094
  • Pipeline finished with Success
    about 1 month ago
    Total: 424s
    #442108
  • 🇺🇸United States tim.plunkett Philadelphia

    Saving credit

  • Pipeline finished with Skipped
    about 1 month ago
    #442123
  • 🇺🇸United States tim.plunkett Philadelphia

    Merged !769 🎉

  • Pipeline finished with Success
    about 1 month ago
    Total: 924s
    #445123
  • Automatically closed - issue fixed for 2 weeks with no activity.

Production build 0.71.5 2024