-
-
Notifications
You must be signed in to change notification settings - Fork 328
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
Toolbar menu #1386
Toolbar menu #1386
Conversation
7740f60
to
9b72731
Compare
@ice0 please have a look |
Looks good to me. @morevnaproject should the command show/hide the toolbar for all windows or only for the selected one? |
If you ask me I think it's more of like preference so if I hide, it should
not be visible in other windows as well.
However as every window have it's own toolbar so I'm not sure if this is
what is expected.
If we go with 1st option then we should not have separate toolbar for every
window.
…On Wed, Apr 29, 2020, 4:47 PM ice0 ***@***.***> wrote:
Looks good to me.
@morevnaproject <https://github.com/morevnaproject> should the command
show/hide the toolbar for all windows or only for the selected one?
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#1386 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AI7ACH7XJGYEBPFVTGSYNLDRPAEDNANCNFSM4MSULROQ>
.
|
Reviewing now! ^__^ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have checked the function - it works well!
Please check one comment regarding function names.
Thank you! ^__^
f1637b5
to
889b3ca
Compare
cf8307e
to
9b72731
Compare
@ARYA-BHARDWAJ Looks good to me now! Can you please rebase your branch on top of current master, so I can make a clean merge? |
@morevnaproject I have rebased the branch on top of current master. I hope this branch can be merged now. |
@ARYA-BHARDWAJ Hmm... I see it is still not rebased yet. Please make sure that you fetched the latest master commits. You can do this with following commands (I assume that you have main synfig repo as "upstream" remote):
|
0a5938b
to
af4b833
Compare
@morevnaproject sorry my bad I forget to push. I hope I have done it correctly |
Merged now. Thank you! ^__^ |
This PR is regarding issue no #733. Now "View" -> "Toolbar" menu item is active.