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

Discussion - Should clearing theme.json customisations also clear Editor Style customisations #115

Open
thatisrich opened this issue Oct 2, 2024 · 2 comments
Labels
bug Something isn't working

Comments

@thatisrich
Copy link
Contributor

thatisrich commented Oct 2, 2024

Behavior in question

After changing the theme Styles via the Editor, should clearing the customisations via Themer be restricted to only changes made in Themer or also cover changes made to the theme via the Editor panel?

For example, it's not currently possible to edit styles such as the colours in the palette via Themer, so clearing customisations of that type via Themer could be confusing.

This may be somewhat related to issue #99.

Some possible outcomes

  1. The current approach is correct, no changes to be made
  2. The current approach is correct, however some more information could be presented to the user prior to clearing changes
  3. Clearing customisations via Themer only removes changes made within the Themer interface
  4. Another solution

Step-by-step reproduction instructions

  1. Go to 'Appearance > Editor'
  2. Edit a page and then click on the Styles icon at the top right of the screen
  3. Make some changes e.g. Colours or Typography
  4. Save the changes
  5. Go to Appearance > Styles Editor
  6. Without making any changes, clear all customisations using the Tools dropdown at the top right of the screen
  7. Save the change
  8. Return to the Editor and notice the Styles changes are reset

Screenshots, screen recordings, code snippets

Demo showing customisations being made via the Styles panel in the Editor, then cleared via Themer

@thatisrich thatisrich added the bug Something isn't working label Oct 2, 2024
@g-elwell
Copy link
Member

g-elwell commented Oct 2, 2024

Great point, thanks for raising this. I hadn't considered this scenario up until now.

I agree, it's confusing to clear styles that aren't currently part of what is editable within themer. Hopefully this will become less of an issue as we move forward and introduce more capabilities. But in the mean time, perhaps we should be limiting what is cleared when you hit that button.

In simple terms, this would be everything under the top-level styles attribute in theme.json. settings is where the colour palette, etc, is stored, which we don't yet support.

@jordanharris-6
Copy link
Contributor

@g-elwell Been discussing this with @thatisrich i've looked into this and we could seperate it so settings are never cleared unless they clear it in the styles editor.

What i've noticed though is that there isn't a way currently to tell if the styles have been edited in the styles editor or themer. When you edit it via the styles it changes it in themer and if you edit it in themer it changes it in the styles editor as it all goes in and uses the same currentGlobalStylesId.

Rich thought it would be a good idea to have a confimation message which says are you sure you want to reset this it will also clear changes made in the styles editor?

Happy to implement these changes as I've already sorted the settings being cleared locally so I could add the confirmation message to the same PR.

Let me know what you think.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants