-
Notifications
You must be signed in to change notification settings - Fork 235
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
feat: Support for Markdown Formatting Keyboard Shortcuts #3936
base: main
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
@Abhi-Bohora is attempting to deploy a commit to the Daily Dev Team on Vercel. A member of the Team first needs to authorize it. |
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.
This is looking great overall. Love how much effort you've put into this, I can see it through how well the code is.
Just had one question, but I will be soon testing this. Also, apologies for the time taken before the initial review 🙏
break; | ||
case 'v': | ||
e.preventDefault(); | ||
handleLinkPaste(); |
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.
Would it not work if you execute onLinkCommand
instead?
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.
@sshanzel Thanks for your review 🙏. I have used onLinkCommand
when the user presses ctrl+l after they have selected the word and wants to insert link manually. In this case executing simply onLinkCommand
would work, pressing ctrl+l will do this [word](url)
. But for ctrl + v it's different if the user have selected the word and have pasted a url
above it than we want to do this [word](https://www.lipsum.com/)
otherwise if it's not a url
than it's a normal paste. so that's why i have extracted the logic in handleLinkPaste();
function. hope this makes sense 😊
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.
Right. Good point. Thank you for elaborating the point! 🚀
Changes
Link: #1624
Describe what this PR does
This PR adds the keyboard shortcuts to format the markdown here are the descriptions of what each shortcut do:
cmd/ctrl+b : set selected text as bold
cmd/ctrl+i : set selected text as italics
(pressing cmd/ctrl+b or cmd/ctrl+i again on the selected text will undo the formatting)
cmd/ctrl+l : replace selected text with text, the author of the issue wanted this command to be cmd/ctrl+k but we already have this shortcut for search i guess, so pressing this command will take the focus to search input field so i decided to change the combination.
cmd/ctrl+v : when pasting a link over selected text, convert to hyperlink
Note
I have tested this on windows and linux haven't tested it on mac
This was fun and challenging for me. As we all know text manipulation is not a fun thing to do....😉
Thank you daily.dev for keeping this awesome project open-source 🙏❤️
Events
Did you introduce any new tracking events?
If yes please remove the comment HTML comment tags and fill the table below
Don't forget to update the Analytics Taxonomy sheet
Log the new/changed events below:
-->
Experiment
Did you introduce any new experiments?
Manual Testing
Caution
Please make sure existing components are not breaking/affected by this PR