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 Option to format via context menu #290

Closed
wants to merge 13 commits into from
Closed

Add Option to format via context menu #290

wants to merge 13 commits into from

Conversation

sibiraj-s
Copy link
Contributor

@sibiraj-s sibiraj-s commented Jul 19, 2020

Closes #20, Partially adresses #21

Screenshot 2020-07-19 at 7 24 09 PM

@kaicataldo
Copy link
Member

Related discussion: #322

@sibiraj-s sibiraj-s mentioned this pull request Aug 23, 2020
@nickmccurdy
Copy link
Member

Thanks for this. I think contex-menu.js should be context-menu.js. Also have you been able to confirm that this works in our supported sites on Firefox, and the existing non-menu functionality still works in Chrome?

@sibiraj-s
Copy link
Contributor Author

I think contex-menu.js should be context-menu.js

AutoComplete 😅 . updated now.

Also have you been able to confirm that this works in our supported sites on Firefox, and the existing non-menu functionality still works in Chrome?

Yes. I verified it. There is no changes to existing code and the context-menu script is loaded only in Firefox.

@kaicataldo
Copy link
Member

Thanks for the PR! I'd like to implement #322 before we add any other functionality to the extension. I'm definitely not against considering this in the future once that's completed, but I think it's best to start with the new approach and see what we feel is missing.

@ghost ghost mentioned this pull request Sep 3, 2020
sibiraj-s and others added 2 commits September 3, 2020 10:18
Base automatically changed from master to main February 1, 2021 03:57
@kaicataldo
Copy link
Member

#322 has been merged. We should re-evaluate this in light of the new behavior.

@sibiraj-s
Copy link
Contributor Author

Thanks @kaicataldo .

RightClick to format is a general option available mostly everywhere. I think it would be nice to include the feature. May be with the current changes. I may be even make it work across the browsers.

@kaicataldo
Copy link
Member

I definitely think it's a good idea!

I wonder if we can add two context menu options:

  • "Format with Prettier"
  • "Format with Prettier as ..."
    • JavaScript
    • Markdown
    • CSS
    • etc.

Thoughts?

@sibiraj-s
Copy link
Contributor Author

Yes It is a nice option. Till we figure out language detection. It will be very handy.

I do remember giving a thought on this when I was working with this feature. I think there is a limitation. An Extension can have only one context menu option. Adding more than one will automatically group them into sub menus.

My memory is little vague on that. I will check and update back

@sibiraj-s
Copy link
Contributor Author

Yes. It is not possible to render more than one context menu item

You can create as many context menu items as you need, but if more than one from your extension is visible at once, Google Chrome automatically collapses them into a single parent menu.

https://developer.chrome.com/docs/extensions/reference/contextMenus/#usage

@sibiraj-s sibiraj-s changed the title Firefox Only - Add Option to format via context menu Add Option to format via context menu Feb 11, 2021
@sibiraj-s sibiraj-s closed this Oct 18, 2021
@sibiraj-s sibiraj-s deleted the add-context-menu branch October 18, 2021 16:38
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.

Generic text box support
3 participants