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.

Sign up for GitHub

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

Jump to bottom

People think the Color Scheme editor will change the current colors #9775

Closed
Tracked by #12400
tony-stines opened this issue Apr 11, 2021 · 12 comments · Fixed by #13269
Closed
Tracked by #12400

People think the Color Scheme editor will change the current colors #9775

tony-stines opened this issue Apr 11, 2021 · 12 comments · Fixed by #13269
Labels
Area-Settings UI Anything specific to the SUI Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. Priority-2 A description (P2) Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Milestone
Terminal v1.17

Comments

@tony-stines
Copy link

Windows Terminal version (or Windows build number)

10.0.21354 (Dev)

Other Software

No response

Steps to reproduce

  1. Launch Terminal
  2. Open Settings
  3. Color Schemes
  4. Select another color scheme from combo
  5. Click to Save
  6. Reopen Settings -> Color Scheme didn't applied

Expected Behavior

No response

Actual Behavior

Color Scheme.
@ghost ghost added Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Tag-Fix Doesn't match tag requirements labels Apr 11, 2021
@DHowett
Copy link
Member

DHowett commented Apr 11, 2021

What terminal version.

@DHowett DHowett added the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Apr 11, 2021
@sphars
Copy link

sphars commented Apr 14, 2021 •
edited
Loading

I'm seeing this issue as well. Just updated to v1.7.1033.0 (the new release). No matter what I try, I can't seem to apply a color scheme using the new Settings UI. It gets reset back to Campbell on each relaunch.

Edit: This issue with with the default color scheme. Changing the scheme under individual profiles does apply the scheme properly to that profile.

Edit 2: Looking at recent commits, looks like it might be fixed in #9764

@miwojc
Copy link

miwojc commented Apr 16, 2021 •
edited
Loading

Same issue. Version: 1.7.1033.0
Edit: Also would be nice to be able to get to settings json file with keyboard shortcut (as before graphical UI ctr ,) because graphical UI doesn't have all the options settings json has. thanks!

@zadjii-msft zadjii-msft mentioned this issue Apr 16, 2021
Closed
@zadjii-msft zadjii-msft added Area-Settings UI Anything specific to the SUI Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-1 A description (P1) Product-Terminal The new Windows Terminal. labels Apr 16, 2021
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Apr 16, 2021
@zadjii-msft zadjii-msft added this to the Terminal v2.0 milestone Apr 16, 2021
@zadjii-msft zadjii-msft mentioned this issue Apr 19, 2021
Closed
@zadjii-msft
Copy link
Member

Wait HOLD UP.

Are you going to the "Color Schemes" page
image

or are you changing the "color scheme" of a given profile:
image

The first page is used for editing the color scheme itself. The second is for changing the appearance of the selected profile. The dropdown on the Color Schemes page is just used for selecting which scheme you want to edit. It doesn't apply that scheme to any profiles.

@sphars
Copy link

sphars commented Apr 20, 2021

@zadjii-msft You're totally correct, now that I've taken a look into that. Gotta say though, it's definitely confusing on first use. I would assume that the Color schemes page is for selecting a default color scheme for all profiles (and yes you could edit the colors as well). Then you can go into individual profiles and override to a different color scheme.

Perhaps it just needs some some clarification on what the Coloring schemes page does. Maybe selecting a default color scheme would be better suited under Appearance (if setting a default color scheme is a goal).

@miwojc
Copy link

miwojc commented Apr 20, 2021

Is it possible to change default theme that would apply to all profiles like via settings.json?

@DHowett
Copy link
Member

DHowett commented Apr 20, 2021

Is it possible to change default theme that would apply to all profiles like via settings.json?

/cc @cinnamon-msft

@zadjii-msft
Copy link
Member

I'll make sure to pass that feedback along to the UI designers (and my "I" I mean @cinnamon-msft 😋)

I'll close the rest of this discussion about setting the default for all profiles as a /duplicate of #9539 - we're generally having that discussion in that thread.

@ghost
Copy link

ghost commented Apr 20, 2021

Hi! We've identified this issue as a duplicate of another one that already exists on this Issue Tracker. This specific instance is being closed in favor of tracking the concern over on the referenced thread. Thanks for your report!

@ghost ghost closed this as completed Apr 20, 2021
@ghost ghost added Resolution-Duplicate There's another issue on the tracker that's pretty much the same thing. and removed Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something labels Apr 20, 2021
@zadjii-msft zadjii-msft mentioned this issue Jun 1, 2021
Closed
@zadjii-msft zadjii-msft added the Needs-Discussion Something that requires a team discussion before we can proceed label Jan 10, 2022
@zadjii-msft zadjii-msft reopened this Jan 10, 2022
@zadjii-msft zadjii-msft removed Resolution-Duplicate There's another issue on the tracker that's pretty much the same thing. Needs-Discussion Something that requires a team discussion before we can proceed labels Jan 10, 2022
@zadjii-msft zadjii-msft removed this from the Terminal v2.0 milestone Jan 10, 2022
@zadjii-msft zadjii-msft mentioned this issue Mar 10, 2022
Merged
4 tasks
@ghost ghost added In-PR This issue has a related PR and removed In-PR This issue has a related PR labels Mar 10, 2022
ghost pushed a commit that referenced this issue Mar 10, 2022
@zadjii-msft
Add some disclaimer text to the schemes page (#12663)
460a991 
People get confused about this. This should help. It doesn't really fix it, but it should help.

* [x] Does enough for #9775 to get it out of 1.14
* [x] I work here
* [x] Screenshot below.

![image](https://user-images.githubusercontent.com/18356694/157732913-86f0af51-8c37-4827-9d21-5775d0bfdeb7.png)

* [ ] todo: Discuss the text here. @cinnamon-msft this sound good?
zadjii-msft added a commit that referenced this issue Mar 10, 2022
@zadjii-msft
Add some disclaimer text to the schemes page (#12663)
addb3a3 
People get confused about this. This should help. It doesn't really fix it, but it should help.

* [x] Does enough for #9775 to get it out of 1.14
* [x] I work here
* [x] Screenshot below.

![image](https://user-images.githubusercontent.com/18356694/157732913-86f0af51-8c37-4827-9d21-5775d0bfdeb7.png)

* [ ] todo: Discuss the text here. @cinnamon-msft this sound good?
@zadjii-msft zadjii-msft modified the milestones: Terminal v1.14, Backlog Mar 15, 2022
@zadjii-msft zadjii-msft added Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. Priority-2 A description (P2) and removed Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-1 A description (P1) zInbox-Bug Ignore me! labels Mar 15, 2022
DHowett pushed a commit that referenced this issue Mar 28, 2022
@zadjii-msft @DHowett
Add some disclaimer text to the schemes page (#12663)
ca93ad3 
People get confused about this. This should help. It doesn't really fix it, but it should help.

* [x] Does enough for #9775 to get it out of 1.14
* [x] I work here
* [x] Screenshot below.

![image](https://user-images.githubusercontent.com/18356694/157732913-86f0af51-8c37-4827-9d21-5775d0bfdeb7.png)

* [ ] todo: Discuss the text here. @cinnamon-msft this sound good?

(cherry picked from commit 460a991)
This was referenced Jun 15, 2022
Open
Open
Closed
@zadjii-msft zadjii-msft modified the milestones: Backlog, Terminal v1.16 Jun 22, 2022
@zadjii-msft zadjii-msft mentioned this issue Jul 29, 2022
Merged
@ghost ghost added the In-PR This issue has a related PR label Aug 1, 2022
@ghost ghost removed the In-PR This issue has a related PR label Aug 25, 2022
@zadjii-msft
Copy link
Member

Punting this for a release, since we feel like this wasn't wholly addressed in #13269

@ghost ghost added the In-PR This issue has a related PR label Aug 31, 2022
carlos-zamora added a commit that referenced this issue Aug 31, 2022
@PankajBhojwani @carlos-zamora
Rejuvenate the color schemes page (#13269)
7076613 
Update the color schemes page with the new Win 11 style.
With the rejuvenated experience, we now have two pages:

- `ColorSchemes.xaml`: "color scheme selection" page
	- This is the starting page for the "color schemes" nav view
	  item. It is intended to have the user select a color scheme
	  they want to modify. The user can also click the "Add new"
	  button to add a new color scheme or the "delete" button to
	  delete the selected scheme.
	- If a scheme cannot be deleted, the delete button is disabled
	  and a disclaimer is shown.
	- A "set as default" button sets the selected scheme as the
	  color scheme for profiles.default (aka "base layer"). 
	- The list view item for each scheme includes the name of the
	  scheme, the default tag (if the scheme is the one set in base
	  layer), a text preview of the foreground/background, and a
	  grid of 16 color chips showing the colors for the scheme.
	- Implementation details:
		- View - `ColorSchemes`:
			- "Enter" --> edit the selected scheme
			- "Delete" --> delete the selected scheme
			- if the selected scheme cannot be deleted, we
			  show the disclaimer
		- View model - `ColorSchemesPageViewModel`
			- when possible, the XAML binds directly to the
			  view model functions. Thus, we include logic
			  to delete, edit, and set the selected scheme
			  as default.
			- store the current page, so that we know which
			  page to navigate to upon saving/discarding
			  changes.
- `EditColorScheme.xaml`: "color scheme modification" page
	- a terminal preview of the color scheme is shown at the top of
	  the page.
	- all colors for the scheme are displayed as color chips in an
	  expander that starts as expanded.
	- renaming a color scheme is also inside an expander, but
	  there's no need for "renaming mode" anymore
	- Implementation details:
		- View - `EditColorScheme`:
			- include logic to display the disclaimer and
			  add the automation properties
			- include logic for "Enter" and "Escape" in the
			  rename editor
		- View Model - `ColorSchemeViewModel`:
			- as before, when possible, the XAML binds
			  directly to the view model functions.
			- To enable the "default" tag functionality, we
			  had to expose knowledge of being the default
			  via "IsDefaultScheme()" which compares the
			  current name to the one in the settings model.
- Miscellaneous implementation details:
	- to get the expander to start as expanded, we had to modify the
	  setting container style.
	- Since "set as default" is a button on the selector page, we
	  needed a way to refresh the view model's knowledge of being
	  the default. So we added a `RefreshIsDefault` API to the
	  `ColorSchemeViewModel` to notify changes.
	- With the new layout, we no longer need an 'enter rename mode'
	  button, so all that logic has been removed
	- Add logic to `MainPage` to handle navigating to the correct
	  page upon saving/discarding changes.

Closes #9775

Co-authored-by: Carlos Zamora <cazamor@microsoft.com>
@ghost ghost added Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. and removed In-PR This issue has a related PR labels Aug 31, 2022
@ghost
Copy link

ghost commented Sep 13, 2022

🎉This issue was addressed in #13269, which has now been successfully released as Windows Terminal Preview v1.16.252.:tada:

Handy links:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Area-Settings UI Anything specific to the SUI Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. Priority-2 A description (P2) Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Projects
None yet
Milestone
Terminal v1.17
Development

Successfully merging a pull request may close this issue.

Rejuvenate the color schemes page microsoft/terminal
5 participants
@DHowett @sphars @zadjii-msft @miwojc @tony-stines