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

feat: removed non-official themes from the settings dialog #1994

Merged
merged 2 commits into from
Apr 14, 2023
Merged

Conversation

kittaakos
Copy link
Contributor

@kittaakos kittaakos commented Apr 4, 2023

## Depends on #1988

Motivation

This PR updates the behavior or the theme <select> in the Settings dialog and makes it overall easier to "install" and use custom (VS Code extension-based) themes in IDE2.

Change description

  1. IDE2 shows only Light, Dark, and High Contrast themes in the Settings dialog.
1283_01.mp4
  1. This PR does not change the fully qualified name of the Arduino themes or the behavior of the Preferences: Color Theme command contributed by Theia. (Or with the Ctrl/⌘+K Ctrl/⌘+T key-chord.) Experienced users should use this command to list and select unofficial themes. The quick-pick theme items show the fully qualified names of the Arduino themes to follow the naming convention among the VS Code extension-based themes. This PR does not change the behavior of the workbench.colorTheme preference.
1283_02.mp4
  1. This PR fixes the location of the default plugin (VS Code extension) drop-in folder. With this, there is no need to copy the custom themes inside the application (Remove deprecated Theia themes #1283 (comment)). It is sufficient to copy the theme VSIX to the shared drop-in folder. If you have unofficial themes in the plugins folder, you can list them and select one with the Preferences: Color Theme command:
1283_03.mp4
  1. The Settings dialog is backward compatible. If you select a non-official theme, it will work after this update. IDE2 will show Unofficial - ${THEME_NAME} as the <select> label.
1283_04.mp4
  1. When an unofficial theme is selected, the Settings dialog shows Unofficial - ${THEME_NAME}. When you unselect the unofficial <select> label, it won't be possible to select it again. Users must cancel the Settings dialog or use a command or the advanced settings to select custom, unofficial themes.
1283_05.mp4
  1. This PR handles obsolete themes. If a VSIX theme is deleted from the drop-in plugins folder, IDE2 detects it on startup and selects a compatible theme from the built-in ones.
1283_06.mp4

Other information

Closes #1283
Closes #1851

Reviewer checklist

  • PR addresses a single concern.
  • The PR has no duplicates (please search among the Pull Requests before creating one)
  • PR title and description are properly filled.
  • Docs have been added / updated (for bug fixes / features)

@kittaakos kittaakos added type: enhancement Proposed improvement topic: code Related to content of the project itself type: imperfection Perceived defect in any part of project topic: theme Related to GUI theming labels Apr 4, 2023
@kittaakos kittaakos self-assigned this Apr 4, 2023
@kittaakos kittaakos force-pushed the fix-update-settings-model-on-theme-change branch from a90a6c7 to cd43a75 Compare April 4, 2023 14:31
@kittaakos kittaakos changed the title #1283 feat: removed non-official themes from the settings dialog Apr 4, 2023
@kittaakos kittaakos force-pushed the fix-update-settings-model-on-theme-change branch from cd43a75 to a431456 Compare April 13, 2023 12:39
@kittaakos kittaakos marked this pull request as ready for review April 13, 2023 12:41
Copy link
Contributor

@per1234 per1234 left a comment

Choose a reason for hiding this comment

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

Users must cancel the Settings dialog or use a command or the advanced settings to select custom, unofficial themes.

Although this is an excellent approach to migrating people away from the deprecated built-in Theia themes (#1283), it harms the user experience when using manually installed themes. It was decided that manual installation of theme extensions will be officially supported (though of course Arduino does not maintain or provide support for the individual 3rd party themes themselves). So we do need to give consideration to that use case.

Would it be possible to apply this behavior (only showing the theme in the "Theme" menu of the "Preferences" dialog if already selected) exclusively to the deprecated built-in Theia themes rather than applying it to the manually installed theme extensions in ~/.arduinoIDE/plugins as well (as is currently done by the PR)? Since it is an officially supported use case, I think the user should be able to select a manually installed 3rd party or custom theme from the "Preferences" dialog (as is the behavior in Arduino IDE 1.x).

Base automatically changed from fix-update-settings-model-on-theme-change to main April 14, 2023 06:56
Closes #1851

Signed-off-by: Akos Kitta <a.kitta@arduino.cc>
@kittaakos
Copy link
Contributor Author

Thank you for the review. I made the requested changes, and the build is running.

Instead of updating the original description of this PR and replacing the screencasts, this section shows the differences between the latest review.

  • User-installed (VSIX) themes are always visible in the Settings dialog theme dropdown.
  • User-installed theme labels have the (user) suffix.
  • If the currentTheme is either Light (Theia) or Dark (Theia), it will be visible from the dropdown with the (deprecated) suffix.
  • The themes in the <select> dictates the following ordering:
    • Light,
    • Dark,
    • Hight Contrast,
    • User-installed themes grouped by light, dark, and hc type. If the type equals, sorting is done by the theme label,
    • Deprecated theme comes last:

Screen Shot 2023-04-14 at 11 04 27

1283.mp4

Copy link
Contributor

@per1234 per1234 left a comment

Choose a reason for hiding this comment

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

Thanks Akos!

Closes #1283
Ref eclipse-theia/theia#11151

Signed-off-by: Akos Kitta <a.kitta@arduino.cc>
@kittaakos
Copy link
Contributor Author

With the most recent changes, the themes are grouped by built-in, user, and deprecated groups. All the sorting and the behavior must be the same.

1283_separator.mp4

Copy link
Contributor

@per1234 per1234 left a comment

Choose a reason for hiding this comment

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

Looks great. Thanks!

@kittaakos kittaakos merged commit 5540170 into main Apr 14, 2023
@kittaakos kittaakos deleted the #1283 branch April 14, 2023 11:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: code Related to content of the project itself topic: theme Related to GUI theming type: enhancement Proposed improvement type: imperfection Perceived defect in any part of project
Projects
None yet
2 participants