Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

move the nav controls to left in full screen mode - in macOS Fix #7872 #8514

Merged
merged 1 commit into from
May 3, 2017

Conversation

kumarrishav
Copy link
Contributor

@kumarrishav kumarrishav commented Apr 26, 2017

Test plan

  1. Launch Brave on macOS
  2. Click the green button in the top left to trigger full screen mode
  3. Notice the back/forward arrows move after the traffic lights disappear
  4. Move mouse into top left corner, hit green button to un-trigger full screen mode
  5. Notice traffic lights appear again and back/forward arrows pop to the right again

Description

  • Submitted a ticket for my issue if one did not already exist.
  • Used Github auto-closing keywords in the commit message.
  • Added/updated tests for this change (for new code or code which already has tests).
  • Ran git rebase -i to squash commits (if needed).

Resolves #7872

Test Plan:
screen shot 2017-04-27 at 12 12 15 am

@bsclifton bsclifton self-requested a review April 26, 2017 20:26
@bsclifton bsclifton added this to the 0.15.1 milestone Apr 26, 2017
Copy link

@cndouglas cndouglas left a comment

Choose a reason for hiding this comment

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

Unfortunately, this causes the back/forward buttons to be underneath the window controls in normal (non-fullscreen) mode.
image

@cndouglas cndouglas added design A design change, especially one which needs input from the design team. OS/macOS labels Apr 27, 2017
@luixxiul
Copy link
Contributor

Please look for something like :not(.fullscreen).

@kumarrishav
Copy link
Contributor Author

kumarrishav commented Apr 28, 2017

@luixxiul How should i check, if screen is in full screen mode or not?
Update : i found it :)

@NejcZdovc
Copy link
Contributor

@kumarrishav so instead of modifying less file, you need to modify some js file. More specifically app/renderer/components/navigation/navigator.js. Here you need to check if window is maximized and if so you need to adjust margin-left for div with class backforward. Here you can see our state file. Checkout isMaximized in ui section.

@kumarrishav
Copy link
Contributor Author

@liunkae review?
screen shot 2017-04-28 at 4 48 42 pm
screen shot 2017-04-28 at 4 48 51 pm

@@ -225,7 +225,7 @@ class Navigator extends ImmutableComponent {
onDragOver={this.onDragOver}
onDrop={this.onDrop}
>
<div className='backforward'>
<div className={'backforward' + (isFullScreen() ? ' fullscreen' : '')}>
Copy link
Contributor

Choose a reason for hiding this comment

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

Please use cx syntax for class name, you can see example bellow. Thank you

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed. I wasn't aware of this utility func. @NejcZdovc Thanks .

Copy link

@cndouglas cndouglas left a comment

Choose a reason for hiding this comment

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

Great job! Works for me. It even animates when you toggle fullscreen.

Copy link
Member

@bsclifton bsclifton left a comment

Choose a reason for hiding this comment

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

Very nice- works great 😄 Well done

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
design A design change, especially one which needs input from the design team. OS/macOS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants