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

feat(ide-extension): message extraction #372

Merged
merged 17 commits into from
Feb 16, 2023
Merged

Conversation

openscript
Copy link
Contributor

@openscript openscript commented Feb 14, 2023

Open tasks:

  • Better error handling when query fails (not just unwrap())

@openscript openscript linked an issue Feb 14, 2023 that may be closed by this pull request
@render
Copy link

render bot commented Feb 14, 2023

@openscript
Copy link
Contributor Author

in #133 the following will be done:

  • manage resources inside the extension state

Copy link
Member

@samuelstroschein samuelstroschein left a comment

Choose a reason for hiding this comment

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

Aweeeesome to see the ide extension working again!

Open questions

Should the callback be used as "name" and "id" be removed from `replacementOptions?

The developer experience could be improved by showing the result of the action instead of an "id".

I extracted a message and was taken back for a few seconds when I got prompted whether I want to "extract a message as JSX or JS string". Answering the question leads to multiple seconds of thinking. The question collides with the result I want to achieve e.g. "reference the message in a t() function that is wrapped in curly brackets" and not "extract as JSX string".

Proposal

  1. Show the result of the callback as a replacement option instead of the id/name in the extract message prompt.
  2. Remove id from the replacement option type but keep the replacement option as an object to allow the later introduction of id, name, or other properties.
CleanShot.2023-02-15.at.17.38.21.mp4

@openscript
Copy link
Contributor Author

Great proposal. I've implemented this:

Kooha-2023-02-16-15-09-33.webm

@samuelstroschein
Copy link
Member

LGTM.

If ready for merge, go ahead and merge this PR :)

@openscript openscript merged commit f220137 into main Feb 16, 2023
@samuelstroschein samuelstroschein deleted the 132-extraction-shortcut branch February 16, 2023 14:28
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.

extraction shortcut
2 participants