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

Feature/dark mode UI v2 #309

Open
wants to merge 92 commits into
base: releaseCandidate
Choose a base branch
from

Conversation

Astisme
Copy link

@Astisme Astisme commented Feb 15, 2024

Describe your changes

Complete overhaul to implement dark mode on every page.
This inlcudes the following:

  • Ability to switch to light/dark mode from within the popup

  • Ability to switch to light/dark mode from the options page

  • Ability to pick between default and accent mode form within the options page

  • Synchronization of theme & accent between any inspector page and any other inspector page (including popup)

  • Load of page with the latest theme and accent (fast-theme.js)

  • New styling for inspector button (need inspector.css in the manifest) when theme != light && accent != default

While I was working on CSS,

  • Found some duplicated entries which I've deleted for easier code readability
  • Stumbled upon a fix for images on the export page to work both on Chrome and Firefox

While I was trying to test on Firefox,

  • Fixed the error which was coming up with links.js not being recognized as a module (removed from the manifest and renamed to links.mjs)
  • Updated the readme.md with the newest command for the sf cli.
  • Found an issue with deploying the Account Settings for relating Accounts and Contacts (this needs fixing)

Issue ticket number and link

Issue #64

Checklist before requesting a review

  • I have read and understand the Contributions section
  • Target branch is releaseCandidate and not master
  • I have performed a self-review of my code
  • I ran the unit tests and my PR does not break any tests
  • I documented the changes I've made on the CHANGES.md and followed actual conventions
  • I added a new section on how-to.md (optional)

tprouvot and others added 30 commits April 21, 2023 12:11
…n for options main page with popup (and viceversa)
@tprouvot
Copy link
Owner

tprouvot commented Apr 8, 2024

Hi @Astisme,
Thanks for the updates 👍

Few remarks, could you :

  • use the existing toggle component to configure the dark theme and accent theme?
    {option: Option, props: {type: "toggle", title: "Enable Dark Mode", key: "enableDarkMode"}},
    {option: Option, props: {type: "toggle", title: "Enable Accent colors", key: "enableAccentColors"}},
  • search and fix every design difference between existing version and your branch ie attached screenshot
  • remove sun and moon icon from popup to keep the configuration on the option page

image

Thank you !

@Astisme
Copy link
Author

Astisme commented Apr 8, 2024

Hi I'm not sure if I'll be able to use the standard implementation of the toggle since I have to do more than simply set a localstorage variable.
Also I'd like to show the user what the default and accent look like before they select it; hence the much different selector for the accents (by doing it this way we may even add more accents in the future and not be limited to 2 of them).

For the other points, I'm still not done aligning the styles.
I'll tell you when it's ready 👍

I thought one could use the sun/moon from the popup to quickly change the color scheme when in other pages (export/ import/ ...) but if you think it's unnecessary then please confirm it and I'll remove it

@tprouvot
Copy link
Owner

Hi @Astisme,
I understand your point regarding the button on the popup but I would prefer to keep the popup without configuration and add dark mode in the option page since it is not something users will update daily.

Regarding the accent options and dark theme, I'm divided into uniformize the option (and maybe update current toggle component to handle other values than true/false) and providing the preview of the accent theme

@Astisme
Copy link
Author

Astisme commented Apr 26, 2024

@tprouvot I need to understand more clearly what you want to achieve with the borders: do you want them to be of the same color aka an even subtler gray?
Because when I started this mini-project I found waaaay too many colors scattered around the files. Here's a snapshot of when I was starting to look for them.
allstyles
and this was the file I was using to match them to the new colors

#000000 --inspector-text
#00396B --inspector-accent
#005FB2 --inspector-primary
#006B2D --inspector-success-background
#006DCC --inspector-accent
#0070D2 --inspector-primary
#009999 --inspector-true-boolean
#009DCC --inspector-accent
#00D9FF --inspector-primary
#0176d3 --inspector-primary
#061C3F --inspector-accent
#1589EE --inspector-primary
#16325C --inspector-accent
#54698D --inspector-neutral
#606061 --inspector-neutral
#61FAB8 --inspector-success
#706E6B --inspector-neutral
#777777 --inspector-neutral
#870500 --inspector-error
#990000 --inspector-false-boolean
#990099 --inspector-number
#999900 --inspector-object
#9FAAB5 --inspector-neutral
#A61A14 --inspector-warning
#AAAAAA --inspector-neutral
#B0ADAB --inspector-neutral
#B0C4DF --inspector-export-background
#C0C0C0 --inspector-neutral
#C23934 --inspector-error
#C9C7C5 --inspector-neutral
#CCCCFF --inspector-working
#D8BE5F --inspector-warning
#D8DDE6 --inspector-neutral
#DDDBDA --inspector-neutral
#E0A4B5 --inspector-import-background
#ECEBEA --inspector-background
#EEEEEE --inspector-background
#EEF1F6 --inspector-backgqound
#EEF6F1 --inspector-background
#EF7EAD --inspector-svg-picture
#F3F3F3 --inspector-background
#F4F6F9 --inspector-background
#F7F7FB --inspector-background
#F7F9FB --inspector-background
#FFEE33 --inspector-warning
#FFFFFF --inspector-background

As you can see, I've tried to simplify the colors for easier maintainability and better understanding.

At the current state of the art, we have 5 different types of gray. Do you want to add more? Do you want me to rename them?

    --inspector-black: #000000;
    --inspector-white: #FFFFFF;

    --inspector-dark-white: #F3F3F3;
    --inspector-very-light-gray: #AFAFAF;
    --inspector-subtle-gray: #C9C9C9;
    --inspector-half-gray: #444444;
    --inspector-other-half-gray: #BBBBBB;

@tprouvot
Copy link
Owner

Hi @Astisme,
My first goal for this PR is to keep the UI and colors as is when the dark mode is not enabled.
I would like to have no differences between current version and the future one with dark mode disabled.
So I would like to keep the border with their original color.

For the simplification of the colors it's a good point and as you said it will be easier to maintain 👍
I'm ok with the naming of the greys, concerning the number, did you remove some ?
Would that mean that we are missing some and we have a difference with current version ? If it's the case I would prefer to keep the current UI. If not I'm ok with the one you provided.

We can have a short meeting to discuss about it I think I would be easier.

@Astisme
Copy link
Author

Astisme commented Apr 30, 2024

Yes, I reduced the number of grays.
Send me a message on discord.
@Astisme

@tprouvot
Copy link
Owner

Hi @Astisme

Friends request seems to be disabled, easiest way would be to add me on Linkedin

image

UnnamedHat added a commit to UnnamedHat/Salesforce-Inspector-reloaded that referenced this pull request May 29, 2024
addon/button.js Outdated Show resolved Hide resolved
addon/button.js Outdated
Comment on lines 247 to 249
if (!window[0].location.pathname.endsWith("popup.html")) {
return setTimeout(() => setupColorChange(error += 1), 500);
}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it still needed since the change color action was removed from popup ?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's needed because the popup also changes colors based on the color/accent scheme

addon/button.js Outdated Show resolved Hide resolved
addon/data-export.js Outdated Show resolved Hide resolved
@Astisme
Copy link
Author

Astisme commented May 31, 2024

Next Steps

  • Align popup to resemble original as close as possible
  • Refactor transitions from inspector.css to the files where the changes wold be applied to
  • Resolve comments before this one
  • Remove styling from slds.css -> create a new custom css file to override the colors

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 this pull request may close these issues.

None yet

3 participants