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

[FancyZones Editor] Add Fluent Design and theming support #8148

Closed
wants to merge 38 commits into from

Conversation

niels9001
Copy link
Contributor

@niels9001 niels9001 commented Nov 19, 2020

Summary of the Pull Request

This PR introduces a brand new UX for the FZ editor.

New features:

  • Ability to duplicate layouts
  • New UX that simplifies the creation and selection of (new) layouts.
  • Theming support (dark/light and all high contrast themes).
  • UI can now be fully controlled with tab / arrow keys
  • Monitor selection now supports keyboard (for selection)
  • Accessibility improvements.
  • Other minor enhancements (e.g. scrollable list).

Things still to be improved / known issues (before merging):

  • Flyout menu should exclude certain properties based on layout type (e.g. templates cannot be deleted)
  • Layout previews do not always update after editing. Needs a 'hard refresh' of the dialog.
  • Settings in layout flyout do not work.
  • Duplicate custom layouts does not work (yet).
  • On mouse leave, the preview should reset to the applied layout.
  • Flyout should appear when applied and right-clicking or using Shift+F10

FZeditor_dark

Editor
image

New layout dialog (heavily inspired by a @zeealeid concept!!!)
image

Layout edit flyout
image

Grid layout editor:
image

Canvas layout editor:
image

Light theme
image

PR Checklist

Info on Pull Request

What does this include?

Validation Steps Performed

How does someone test & validate?

@niels9001 niels9001 added FancyZones-Editor Issue revolving the FancyZone Editor Area-User Interface things that regard UX for PowerToys labels Nov 19, 2020
@yuyoyuppe yuyoyuppe force-pushed the users/niels9001/editor-fluent branch from 6d4f6e1 to 22957c4 Compare November 20, 2020 11:47
@yuyoyuppe
Copy link
Contributor

rebased on the current master

@enricogior
Copy link
Contributor

@niels9001
we had to rebase your current PR to unblock CI.
Before making more changes, you will need to pull from remote to update your local branch.

@enricogior
Copy link
Contributor

@niels9001
you need to remove MahApps.Metro.dll from installer\PowerToysSetup\Product.wxs line 375:

installer\PowerToysSetup\Product.wxs(375,0): Error LGHT0103: The system cannot find the file 'D:\a\1\s\installer\PowerToysSetup\..\..\x64\Release\modules\FancyZones\MahApps.Metro.dll'.

@htcfreek
Copy link
Collaborator

htcfreek commented Dec 2, 2020

Feedback:

  1. The dark color sheme of the "New Layout Dialogue" doesn't fit to the other dialoges.
  2. We should show the text on the reset button too. - The first time I saw the symbol, I thought it was a reload button.
  3. I don't like the additional "settings" menu. But unfortunately at the moment I have no idea how to make it better. Maybe a centered dialog overlay could help.
  4. We should add a menu indicator like the three dots on the layouts.
  5. What about renaming "templates" to "default layouts". Imo templates can't be applied and only used for create new one whereas "defaut layouts" can be applied and used as templates. (It's only an idea.)
  6. We could add a hint for the dup feature in the create new dialogue. (ℹ️ You can create a new layout based on one of our default layouts by using its context menu.)
  7. As suggested in an other issue, we should have a setting to choose the colour mode or we should use the PT settings colour mode.
  8. Grid/Canvas dialogue: Does "Save & Apply" means saving the layout and applying it on the monitor? If yes imo we should only save it and not aply it. - Reason: The user has to select the monitor before creating the layout and which user knows this?
  9. Enable the user to duplicate custom layouts too. (Then the text in point 6 should say "based on an existing layout".)

@niels9001
Copy link
Contributor Author

test

@htcfreek
Copy link
Collaborator

htcfreek commented Dec 4, 2020

The spaces are not saved per layout. They are saved per monitor. So we should split the settings for the layout and the space.

@enricogior
Copy link
Contributor

@htcfreek

The spaces are not saved per layout.

The current implementation is broken, we are going to fix it allowing to edit the spaces per layout.

@htcfreek
Copy link
Collaborator

htcfreek commented Dec 5, 2020

@htcfreek

The spaces are not saved per layout.

The current implementation is broken, we are going to fix it allowing to edit the spaces per layout.

Okay. Then we have an other situation and @niels9001's layout makes sense.

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

@niels9001

The new Editor is too large when I tested it on my small laptop. The bottom part of the Editor was not visible

@niels9001
Copy link
Contributor Author

@alannt777 Can you take a screenshot? We might want to make the dialog (somewhat) resizeable. Still working on that.

@ghost
Copy link

ghost commented Dec 5, 2020

See this:

FZ Editor screenshot

@ghost
Copy link

ghost commented Dec 5, 2020

@niels9001 I have noticed a bug where if you have only one monitor, you can't edit the layouts

@ghost
Copy link

ghost commented Dec 5, 2020

You can't right-click on any layouts at all

@niels9001
Copy link
Contributor Author

@alannt777 please note that this is still in draft and work in progress. There are still bugs I'm actively working on.

@ghost
Copy link

ghost commented Dec 5, 2020

I know 😄

I was just letting you know

@Jay-o-Way
Copy link
Collaborator

Jay-o-Way commented Dec 6, 2020

@htcfreek had a few very good points, I actually agree with all of them!
About the reset icon: you actually did use the "refresh" icon. E7A7 "undo" would be better, text even better.
And will there be a link to the PT Settings too, like #7408?

@htcfreek
Copy link
Collaborator

htcfreek commented Dec 6, 2020

Additional idea:
(Follow up my list above.)

  1. Multi-mon environment: Maybe it's helpful to have a small symbol overlay on the monitor rectangle to indicate, that a layout is applied on this monitor. @niels9001, I think on using the fz symbol from settings' nav bar.

@niels9001 niels9001 mentioned this pull request Dec 7, 2020
16 tasks
@niels9001
Copy link
Contributor Author

@htcfreek @Jay-o-Way I have made some changes to the initial proposal based on your (and @crutkas) feedback. See updated images in original post.

@htcfreek
Copy link
Collaborator

@niels9001 @SeraphimaZ
in single monitor mode we have a usability issue that needs to be fixed: you can only edit the layout properties if the layout is selected
image

For all other layouts is not possible to edit the properties without first applying the layout (that closes the editor) then reopening the layout to edit that layout.
My suggestion is that we don't close the editor when applying a layout in single monitor (same as for multi monitor), so it will be possible to edit the properties of a layout before closing the editor.

Can we hide the grey space at the top in single monitor mode? I think it looks like a bug where some ui elements are missing or not shown.

@enricogior
Copy link
Contributor

This PR depends on #8779 in order to re-enable the file watcher reload the layout settings.

@SeraphimaZykova
Copy link
Collaborator

SeraphimaZykova commented Dec 30, 2020

The work remaining:

P0: The create new layout dialog renders semi-off the screen (depends on Kinnara/ModernWpf@d5d2266)
P1: On mouse leave of the layout item, the preview should reset to the applied layout.
P2: Move zones + and - buttons to the template layouts flyout (release vNext++)
P2: Disable layouts and add information tooltip describing that the layout cannot be applied to the current screen resolution.

@enricogior
Copy link
Contributor

@SeraphimaZ
regarding:

P1: On mouse leave of the layout item, the preview should reset to the applied layout.

We should consider disabling the preview in the first place, since it's not really useful and can cause confusion since the current UI doesn't let change any setting for a preview layout.

@enricogior
Copy link
Contributor

@SeraphimaZ

P2: Move zones + and - buttons to the template layouts flyout (release vNext++)
P2: Disable layouts and add information tooltip describing that the layout cannot be applied to the current screen resolution.

Those two should not be considered a blocked to merge this PR.

@htcfreek
Copy link
Collaborator

htcfreek commented Jan 2, 2021

  • Do I have to apply a layout befor I can see the edit symbol? How is it handled at the moment?

  • In addition to my comment above: Why do we hide the monitor preview in single monitor mode? Can we show only the one existing monitor (as selected) in single monitor mode. Then we don't have lost space.

@enricogior
Copy link
Contributor

enricogior commented Jan 2, 2021

@htcfreek
this is still work in progress and we are considering significant changes.

Do I have to apply a layout befor I can see the edit symbol? How is it handled at the moment?

This is one of the changes that we already decided to do.

Why do we hide the monitor preview in single monitor mode?

We will not.

@htcfreek
Copy link
Collaborator

htcfreek commented Jan 2, 2021

@htcfreek
this is still work in progress and we are considering significant changes.

Do I have to apply a layout befor I can see the edit symbol? How is it handled at the moment?

This is one of the changes that we already decide to do.

👍 Oh. I didn't read the quoted text in my comment above.

Why do we hide the monitor preview in single monitor mode?

We will not.

Maybe we missunderstood each other. But on some images in previous comments there is no monitor symbol in the top space of the window.

@niels9001
Copy link
Contributor Author

@htcfreek as @enricogior pointed out, there will be changes. That's one of the reasons this PR is still in draft.

@enricogior
Copy link
Contributor

@htcfreek

Maybe we missunderstood each other. But on some images in previous comments there is no monitor symbol in the top space of the window.

Because it was a partial screenshot.

@Jay-o-Way
Copy link
Collaborator

Jay-o-Way commented Jan 26, 2021

I think we need to change the term "Duplicate" to something else, since this action creates a custom layout from the template, it doesn't duplicate the template.

Strictly speaking you are correct, "duplicate" means "to make an exact copy of something". In practice, it often means "duplicate-and-then-edit". Seen this in outlook.com where you can "duplicate" an agenda-item. Not saying you don't have a point, but for me functionality has higher priority.
Having that said, if you feel this word should be changed, you should also rename the label "Resolution-duplicate".

@niels9001
Copy link
Contributor Author

Closing this in favor of #9325

@niels9001 niels9001 closed this Jan 27, 2021
@crutkas crutkas deleted the users/niels9001/editor-fluent branch January 27, 2021 18:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-User Interface things that regard UX for PowerToys FancyZones-Editor Issue revolving the FancyZone Editor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants