-
Notifications
You must be signed in to change notification settings - Fork 32
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 keyboard shortcuts for icons in the editor #1962
Add keyboard shortcuts for icons in the editor #1962
Conversation
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.
Congratulation for your first PR in our repository 🎉 It's almost there 😃
Additionaly to the suggestion in the code, I would like you to add an entry into CHANGELOG file under UNRELEASED.
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.
Very cool! Thank you 😃
After squashing the commits (and resolving conflicts if any occurs) I think this PR is ready to be merged 👍
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.
Looks good to me, works as expected. Congrats!
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.
Thanks a lot, really cool feature! 👍
After squashing the commits
I agree, could you execute:
git rebase --interactive HEAD~2
and replace the second "pick" by "fixup" and save?
Then, you'd have to do:
git push --force-with-lease
To overwrite the existing commits.
Additionaly to the suggestion in the code, I would like you to add an entry into CHANGELOG file under UNRELEASED.
Again, I agree, and this change has not yet been addressed.
Could you add the lines:
* [ [#1957](https://github.com/digitalfabrik/integreat-cms/issues/1957) ] Add keyboard shortcuts for icons in the editor
to the CHANGELOG.md into the UNRELEASED
section?
cde9863
to
225e6a6
Compare
a02666a
to
db3221b
Compare
- Refactor the insert icon flow - Replace addIcon with the refactored flow for shortcuts
db3221b
to
b9e895f
Compare
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.
Thanks a lot! 👍
I rebased your branch one last time and cleaned up the git history, now I think this is good to go! 🎉
(One optional idea I had was to somehow document this possibility, otherwise users don't know the keyboard combinations for the icons, but I guess we can also put this in our usage wiki.)
Short description
Add shortcuts for Adding icons in the editor.
Proposed changes
Alt+|+<number>
would insert the corresponding iconspin
, 2 forwww
, etc.Side effects
Resolved issues
Fixes: #1957
Pull Request Review Guidelines