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

Add AppMenu button to headerbar #2132

Merged
merged 15 commits into from
Feb 12, 2023
Merged

Add AppMenu button to headerbar #2132

merged 15 commits into from
Feb 12, 2023

Conversation

jeremypw
Copy link
Collaborator

@jeremypw jeremypw commented Feb 9, 2023

Fixes #2123

  • Zoom controls
    • Update label of "normal" button (?)
    • Make zoom insensitive when at max/min zoom
  • Undo/Redo controls
  • Global boolean options
    • Selection mode for folders
    • Show/hide hidden files
    • Show/hide remote thumbnails
    • Show/hide local thumbnails
    • Sort directories first
  • Remove corresponding context menu items (TBD)

Left for possible future PRs

Screenshot from 2023-02-11 21 01 40
Screenshot from 2023-02-11 21 00 37

@jeremypw
Copy link
Collaborator Author

jeremypw commented Feb 9, 2023

@danirabbit A few questions to resolve regarding this PR:

  • How to handle labelling of "normal zoom" button. Should it change like in Code and Terminal, in which case how to denote the icon size relative to "normal"? Or is it OK to leave it with a fixed label or icon?
  • What is optimum order of the boolean option switches.
  • Is there a need/demand to override the system style in Files? Terminal and Code widgets have their own built-in style handling but for Files this is not the case.
  • Is it OK to remove the corresponding menuitems from the directory context menu?

Please apply your design expertise to the appearance.

@danirabbit
Copy link
Member

@jeremypw this looks awesome!

How to handle labelling of "normal zoom" button. Should it change like in Code and Terminal, in which case how to denote the icon size relative to "normal"? Or is it OK to leave it with a fixed label or icon?

I think using percentages still makes sense here since we have some views with lots of options that saying like "extra small" or "extra large" starts to get meaningless. I think we should still adjust the label to match what is in the view to be consistent here.

What is optimum order of the boolean option switches.

I proposed #2133 with a few suggested changes to organization/structure/appearance

Is there a need/demand to override the system style in Files? Terminal and Code widgets have their own built-in style handling but for Files this is not the case.

I don't think so? The reason that Code/Terminal have these manual overrides is because they're using syntax highlighting schemes where people have pretty strong preferences about using dark/light syntax highlighting separate from the rest of the system. I'm not sure that similar demand exists for other apps

Is it OK to remove the corresponding menuitems from the directory context menu?

Yes! I think we should definitely clean those up so that menu can become simpler

@danirabbit
Copy link
Member

Ah something else I noticed is that the zoom actions don't become disabled when they git their min/max values. So for example, the zoom in button is still sensitive even when we're at the max zoom and pressing it has no effect

@jeremypw
Copy link
Collaborator Author

jeremypw commented Feb 9, 2023

@danirabbit Thanks for the comments! I'll get on with implementing those tomorrow.

@danirabbit
Copy link
Member

Oh, actually I'm curious about the "Sort Folders Before Files" option. Is there a reason you wanted to move that here instead of having it with the other sort options in the context menu?

@jeremypw
Copy link
Collaborator Author

Oh, actually I'm curious about the "Sort Folders Before Files" option. Is there a reason you wanted to move that here instead of having it with the other sort options in the context menu?

This is because that is a global option affecting all folders and the file chooser. The other sort options only affect the individual folder you set them on (because in that case they are stored in the file metadata).

@danirabbit
Copy link
Member

I think that one still makes sense to have in the context menu with the other sort options, even if we duplicate it in both locations. But just to make sure sorting options are all available at once and someone won't be confused about where to find this when they're changing other sorting options

@jeremypw
Copy link
Collaborator Author

I think that one still makes sense to have in the context menu with the other sort options, even if we duplicate it in both locations. But just to make sure sorting options are all available at once and someone won't be confused about where to find this when they're changing other sorting options

I have no problem with duplicating it in the context menu.

@jeremypw jeremypw marked this pull request as ready for review February 11, 2023 20:59
Copy link
Member

@danirabbit danirabbit left a comment

Choose a reason for hiding this comment

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

It looks like the "local-thumbnails" check is always disabled when files starts. Somewhere along the way action state isn't being set on startup

@jeremypw
Copy link
Collaborator Author

It looks like the "local-thumbnails" check is always disabled when files starts. Somewhere along the way action state isn't being set on startup

Seems that was a pre-existing bug that this PR exposes. Now fixed.

@jeremypw jeremypw requested a review from danirabbit February 12, 2023 09:32
Copy link
Member

@danirabbit danirabbit left a comment

Choose a reason for hiding this comment

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

Let's go! 🚀

Great work :)

@danirabbit danirabbit enabled auto-merge (squash) February 12, 2023 16:51
@danirabbit danirabbit merged commit 917aa87 into master Feb 12, 2023
@danirabbit danirabbit deleted the appmenu branch February 12, 2023 16:53
This was referenced Feb 12, 2023
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.

Files should have a settings/cog wheel menu in the headerbar
2 participants