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

enhance(toolbar): use native operating system toolbar #1120

Merged
merged 109 commits into from
Jun 13, 2021

Conversation

shanberg
Copy link
Collaborator

@shanberg shanberg commented May 6, 2021

Note: I can't get script/lint and cljstyle fix to agree with each other.

  • Refreshes the toolbars to look more at home on 3 major OSs.
  • Replaces Electron/Browser-style zooming with an internally controlled facsimile, mainly to prevent the toolbar from breaking when you zoom, but also allowing us to provide additional UI helpers for zooming
  • Provides example code for doing more with the Electron wrapper by getting IpcMain events and sending events through channels.
  • Adds app-classes helper to style elements based on electron v browser, and per-OS
  • The styles themselves are mostly a first pass. Main task of this PR is to create the infrastructure for further improvements.

Before
image

After
Screen Shot 2021-05-05 at 11 15 16 PM

tangjeff0 and others added 30 commits July 2, 2020 16:13
@shanberg shanberg marked this pull request as ready for review May 23, 2021 16:58
Copy link
Collaborator

@tangjeff0 tangjeff0 left a comment

Choose a reason for hiding this comment

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

One small-ish issue is that you cannot see the native controls when Athens is a small window for Windows.

Windows

Old

image

New:

image

Mac

New

Mac is fine

Screen Shot 2021-06-09 at 7 04 25 PM

Ubuntu

My guess is this probably also exists for Ubuntu. Unfortunately, mine comes with built-in tiling, so I cannot make the width small. @sid597 can you test this out? Please checkout @shanberg's branch, make the width of Athens small, and then share a screenshot. My guess is that the top-right controls will be hidden.

src/cljs/athens/style.cljs Show resolved Hide resolved
Comment on lines +196 to +201
{:class (app-classes {:os os
:electron? electron?
:theme-dark? @theme-dark
:win-focused? @win-focused?
:win-fullscreen? @win-fullscreen?
:win-maximized? @win-maximized?})})
Copy link
Collaborator

Choose a reason for hiding this comment

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

We may want {:class (...)} to a direct argument to merge, otherwise stylefy will generate class for each permutation of classes.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@tangjeff0 Not sure whether that's possible, since Stylefy has to wrap the props passed to the element.

Comment on lines +288 to +293
{:class (app-classes {:os os
:electron? electron?
:theme-dark? @theme-dark
:win-focused? @win-focused?
:win-fullscreen? @win-fullscreen?
:win-maximized? @win-maximized?})})
Copy link
Collaborator

Choose a reason for hiding this comment

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

We may want {:class (...)} to a direct argument to merge, otherwise stylefy will generate class for each permutation of classes.

@@ -86,7 +88,12 @@

:else [:<>
(when @modal [filesystem/window])
[:div (stylefy/use-style app-wrapper-style)
[:div (use-style app-wrapper-style
{:class [(case (get-os)
Copy link
Collaborator

Choose a reason for hiding this comment

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

May want to put (get-os) in the let binding at the top, otherwise will get computed on each re-render. (unlikely OS of Athens needs to be recomputed)

@sid597
Copy link
Collaborator

sid597 commented Jun 9, 2021

@tangjeff0 Yeah the controls are hidden. Which tiling manager do you use ? I use i3wm and I can make a window float.
Screenshot from 2021-06-10 05-02-22

@tangjeff0
Copy link
Collaborator

tangjeff0 commented Jun 9, 2021

@sid597 I forgot I can make floating windows 🤦 yeah but getting same thing. Changes requested for @shanberg to make the native toolbar controls always visible for Ubuntu and Windows (which are found on the right side, rather than left as in Mac)

@shanberg
Copy link
Collaborator Author

One small-ish issue is that you cannot see the native controls when Athens is a small window for Windows.

@tangjeff0 I've hidden the athena button label at small App widths, and adjusted the app's minimum width to make sure the controls are always visible. Tested on macOS, Windows, Ubuntu.

@shanberg shanberg requested a review from tangjeff0 June 10, 2021 01:50
Copy link
Collaborator

@tangjeff0 tangjeff0 left a comment

Choose a reason for hiding this comment

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

I added ^:cljstyle/ignore to (superficially) address issue of kondo and cljstyle fighting with each other.

I increased minWidth from 670 to 800 because going to 670 width still led to the X button on Ubuntu getting hidden. I used 800 because that was the value I saw in app-toolbar. Maybe that should be parameterized, assuming they should be the same?

src/cljs/athens/style.cljs Show resolved Hide resolved
@tangjeff0 tangjeff0 changed the title style(toolbar): more-native toolbar styles in windows, macos, linux enhance(toolbar): more-native toolbar styles in windows, macos, linux Jun 13, 2021
@tangjeff0 tangjeff0 changed the title enhance(toolbar): more-native toolbar styles in windows, macos, linux enhance(toolbar): use native operating system toolbar Jun 13, 2021
@tangjeff0 tangjeff0 merged commit e2c953b into athensresearch:main Jun 13, 2021
korlaism pushed a commit to korlaism/athens that referenced this pull request Jul 19, 2021
…#1120)

* feat(blocks): update writing smoothly

* refactor(dropdown): separate component defn from defcard

* feat(blocks): hook up slash commands and context menu to blocks

* feat(slash-menu): improved layout

* feat: basic frameless etc stuff in place

* chore: update from main

* fix: add toolbar drag handle

* feat: add minimize button

* feat: add minimize button

* chore: midflight making window minimize button

* feat: working minimize button

* fix: block in windows window controls

* feat: improved style for toolbar buttons on win

* chore: add type hints to graph

* feat: electron min max win toggle effect

* fix: windows toolbar controls work work properly

* feat: rerender max/restore icons

* feat: main view classed with os and electron

* chore: update stylefy

* feat: basic toolbar style conditions in place

* fix: dont hide win controls on win

* feat: improvements to toolbar

* feat: improved style for macos toolbar

* feat: improved style on win

* feat: improved style for macos toolbar

* feat: proper handling of fullscreen in macos

* feat: add conditions for fullscreen windows button

* feat: block in new listener for f11 fullscreen event

* feat: fullscreen and close functionality for win

* feat: toolbar styled for ubuntu

* fix: properly drop minimize icon in linux

* chore: fix code style issues

* feat: shorter titlebar when app is fullscreen

* chore: fix linter issue

* chore: fix linter complaint

* chore: remove unnecessary changes

* fix: remove redundant db attribute

* rfct: use case instead of cond for app view classname

* rfct: simplify app toolbar code

* chore: fix linter complaints

* fix: remove unused import

* feat: add is-focused class to toolbar

* chore: fix linter issues

* feat: improved style for unfocused app toolbar in linux

* chore: revert stylefy upgrade

* feat: midfight adding zoom-factor to db

* feat: build app-level zoom factor

* fix: move base font size to root

* feat: working controlled zoom

* rfct: prefer relative font sizes and no 'all' transitions

* feat: fully control zoom across app

* feat: improved zoom clamping

* chore: remove redundant type

* feat: clearer comment around old zoom cotrols

* fix: solve issue with menu on pc

* feat: better toolbar height for windows

* fix: menu working on pc

* fix: revert code style changes

* chore: better code style in electron

* fix: cljstyle

* fix: revert whitespace change

* fix: remove redundant style

* fix: cleaner code style

* fix: cljstyle

* fix: code style

* fix: code tsyle

* fix: code style

* fix: code style

* fix: contains had extra parens

* fix(left-sidebar): clamp size of version indicator

* fix: page menu follows page title

* rfct: improved zoom range

* chore: fix lint issues

* chore: sort requires

* feat: comment out more unused menu tiems

* rfct: simplify menu to use more defaults

* chore: fix linter issues

* chore: fix linter issue

* refactor: move min-max zoom sizes into style

* chore: misc cleanup

* cleanup: minor refactor and cleanup

* chore: fix linter complaints

* fix: sort namespaces

* fix: fix linter again

* rfct: clean up app classes util

* rfct: centralize ipcMainChannels def

* chore: fix linter issues

* feat: border under toolbar only shown on hover

* fix: proper button size in windows

* feat: hide athena button label at small widths

* feat: increased min app width to toolbar break point

* rfct: minor cleanup

* style: better code comments

* fix: ignore cljstyle and increase minWidth

* single semi-colon inline comments for newest version of cljstyle

Co-authored-by: Jeffery Tang <tangj1122@gmail.com>
Co-authored-by: shanberg <98312+shanberg@users.noreply.github.com>
Co-authored-by: Dominic Chiampi <dominicchiampi@gmail.com>
@shanberg shanberg deleted the frameless-styled-toolbar-3 branch September 27, 2021 23:10
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.

6 participants