Skip to content
This repository has been archived by the owner on Jan 6, 2023. It is now read-only.

Toolbar button for Notes #515

Merged
merged 4 commits into from
Jan 5, 2018
Merged

Toolbar button for Notes #515

merged 4 commits into from
Jan 5, 2018

Conversation

cedricium
Copy link
Collaborator

@cedricium cedricium commented Dec 17, 2017

Fixes #535

Initial start at implementing toolbar button to open Notes directly. Currently, if the sidebar is closed and the Notes browser action (toolbar button) is clicked, the sidebar will open with Notes as the active page.

Refs #535

To-Do:

  • Add active state to toolbar button if Notes is currently open (similar to sidebar toolbar button) -> not sure if possible
  • Add toggle ability to toolbar button:
    • if Notes is open and toolbar button is clicked, Notes/sidebar should close
    • if Notes/sidebar is closed and toolbar button is clicked, Notes should open
  • Get design approvals (specifically regarding toolbar button icon) from @ryanfeeley

Example:
notes_toolbarbtn_demo


// Handle onClick event for the toolbar button
browser.browserAction.onClicked.addListener(() => {
browser.sidebarAction.open();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Don't we have a toggle sidebar Action ?

Copy link
Collaborator Author

@cedricium cedricium Dec 17, 2017

Choose a reason for hiding this comment

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

@Natim I'm not sure what you mean - are you referring to the Sidebar browser action button that looks like this: https://design.firefox.com/icons/viewer/#sidebar?

If so, that toggles the sidebar, but will open whatever page was last opened in the sidebar (so potentially Bookmarks, History, or Notes open). This PR is supposed to add the toolbar button which opens only opens Notes. Let me know if that's what you meant or not.

@ryanfeeley
Copy link
Contributor

👍

@johngruen
Copy link
Contributor

@cedricium you can add light and dark theme icons like this: https://github.com/mozilla/speaktome/blob/master/extension/manifest.json#L495

unfortunately they'll need to be PNG.

@vladikoff
Copy link
Contributor

@cedricium We are back from holiday vacations! woo! did you have time to update the icons to support dark and light themes? Otherwise we are full steam ahead to merge this fro the next release!

@vladikoff
Copy link
Contributor

unfortunately they'll need to be PNG.

@johngruen the code you linked is using svgs, updated?

Create new png for src/icons/notes-**.png based on Photons icons styling
https://design.firefox.com/icons/viewer/
@sebastienbarbier
Copy link
Collaborator

sebastienbarbier commented Jan 4, 2018

@vladikoff @cedricium Hope this will help, just updated manifest to implement theming on browser_action icons.

theming-notes-icon

…open sidebar

Keep state in background.js with global variable.
Should be refactored with non globale during React integration.
@Natim
Copy link
Collaborator

Natim commented Jan 4, 2018

Add active state to toolbar button if Notes is currently open (similar to sidebar toolbar button) -> not sure if possible

How would we like it to work? Should the icon turn blue when open? @ryanfeeley any idea about that?

@Natim Natim changed the title WIP: Toolbar button for Notes Toolbar button for Notes Jan 4, 2018
Use a blue icon if Notes add-on is active.
@sebastienbarbier
Copy link
Collaborator

Tryed to solve active state on browserAction icon using a blue icon. Not sure if this follow any guideline, need to be discussed/defined.

toggle-notes-icon

@Natim Natim merged commit 6545bbf into mozilla:master Jan 5, 2018
@vladikoff
Copy link
Contributor

@vladikoff @cedricium Hope this will help, just updated manifest to implement theming on browser_action icons.

woa awesome!!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add toolbar button to open note sidebar
6 participants