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 new note icon to top toolbar when in focus mode #2596

Merged
merged 6 commits into from
Feb 1, 2021

Conversation

sandymcfadden
Copy link
Contributor

Fix

Adding the new note icon to the note toolbar when we are in focus mode.
This addresses part of the issue #2563 as part of the UI Spring Cleaning.

Screen Shot 2021-01-25 at 3 01 04 PM
Screen Shot 2021-01-25 at 3 01 26 PM

Test

  1. Enter focus mode by toggling the left sidebar so it is hidden
  2. Click the new note icon in the upper left
  3. Ensure the new note is created
  4. Try this in both light and dark mode to ensure styles appear correct.

Release

  • Add the new note icon to the toolbar when in focus mode.

@sandymcfadden sandymcfadden self-assigned this Jan 25, 2021
@sandymcfadden sandymcfadden requested a review from a team January 25, 2021 19:11
Copy link
Member

@codebykat codebykat left a comment

Choose a reason for hiding this comment

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

Looks and works great! There's a teensy alignment issue on the Mac build (I think we're adding some extra toolbar padding for this platform):

Screen Shot 2021-01-25 at 12 06 30 PM

I'm also wondering (and this is an honest question, not me being Socratic) if there's a way to use the same button in the note toolbar and in the menu bar, and use styles to change its placement... or even if those two menu bars should now be the same component. 🤔

Does that seem possible or too much of a pain?

lib/note-toolbar/index.tsx Outdated Show resolved Hide resolved
lib/note-toolbar/index.tsx Outdated Show resolved Hide resolved
@sandymcfadden
Copy link
Contributor Author

Thanks for pointing me in the right direction with the padding @codebykat and catching that. I had tested on macOS and then made some changes and obviously didn't check closely again.

I did take a quick look to see before I started to see if I could use that same button but didn't see a good way to do it. I didn't look at refactoring to make the menu bars one component though. I'll investigate that a bit.

@sandymcfadden
Copy link
Contributor Author

@codebykat I took a look at moving the two menus into one component. I think it could be done. It would likely make for a big component and refactor the layout of the app. I don't think it is something I could do as part of this PR, but would be happy to look at it more together at some point if you are up for it.

lib/note-toolbar/index.tsx Outdated Show resolved Hide resolved
@sandymcfadden sandymcfadden added this to the 2.7.0 milestone Jan 27, 2021
@sandymcfadden sandymcfadden force-pushed the add/new-note-icon-focus-mode branch from 083520f to fd4864a Compare January 29, 2021 12:23
Copy link
Member

@codebykat codebykat left a comment

Choose a reason for hiding this comment

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

Looks and works great now, including on OSX 👍

@sandymcfadden sandymcfadden merged commit a2be8eb into develop Feb 1, 2021
@sandymcfadden sandymcfadden deleted the add/new-note-icon-focus-mode branch February 1, 2021 11:43
@pachlava
Copy link
Contributor

[testing] fix/feature verified on Dell Latitude 7400 | Win 10 | Simplenote 2.7.0-beta1

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.

4 participants