Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Update color-picker extension #17593

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

ridemountainpig
Copy link
Contributor

Description

Add the color format dropdown option to the Convert Color command, to let user can direct select the format they want to convert.
Close #17553

Screencast

Raycast 2025-03-06 at 14 26 13

Checklist

@raycastbot raycastbot added extension fix / improvement Label for PRs with extension's fix improvements extension: color-picker Issues related to the color-picker extension OP is contributor The OP of the PR is a contributor of the extension labels Mar 6, 2025
@raycastbot
Copy link
Collaborator

raycastbot commented Mar 6, 2025

Thank you for your contribution! 🎉

🔔 @thomaspaulmann @otakustay @EmersonEmerson @pernielsentikaer @arronhunt @kvdo2 @thomaslombart @LitoMore @samuelkraft @xilopaint @anwarulislam you might want to have a look.

You can use this guide to learn how to check out the Pull Request locally in order to test it.

Due to our current reduced availability, the initial review may take up to 10-15 business days

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

This PR adds a color format dropdown option to the Convert Color command in the color-picker extension, allowing users to directly select their desired output format.

  • getSelectedText() in /extensions/color-picker/src/convert-color.tsx should be wrapped in try-catch for graceful error handling
  • Could simplify error handling in getConvertedColor() by using showFailureToast from @raycast/utils instead of manual showToast
  • The [Extract Color] changelog entry from 2025-02-25 is missing the required {PR_MERGE_DATE} template string
  • The new format dropdown argument in package.json provides a comprehensive set of color format options matching the existing preferences

5 file(s) reviewed, 2 comment(s)
Edit PR Review Bot Settings | Greptile

"name": "format",
"type": "dropdown",
"placeholder": "Convert Format",
"required": true,
Copy link
Contributor

Choose a reason for hiding this comment

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

style: Consider making the format argument optional and defaulting to the user's preferred format from preferences

Copy link
Collaborator

Choose a reason for hiding this comment

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

I kind of like this idea: having a default in preferences so you don't have to set it every time. What do you think, @LitoMore?

Copy link
Contributor

Choose a reason for hiding this comment

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

That sounds like a good idea because we can make the default value for the last selected option. But the problem is the option title is hardcoded in the package.json. We don't have a proper way to show the name of the last selection.

Copy link
Contributor Author

@ridemountainpig ridemountainpig Mar 6, 2025

Choose a reason for hiding this comment

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

I kind of like this idea: having a default in preferences so you don't have to set it every time.

I like this idea too, but the argument dropdown will store the most recently used value.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think that it does, just tested in my Figlet command and it's resetting, that why I did this

image

CleanShot 2025-03-06 at 10 12 06@2x

Copy link
Collaborator

Choose a reason for hiding this comment

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

Hmmm, looking briefly through some code, it seems like it actually saves the last value. I was able to restart Raycast, and the value is persisted.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I just checked again. It only persisted for a short amount of time

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have looked again after a long time, but the last value still exists. 🤔
If it really only persisted for a short time, I think I can change the required attribute to false.

Copy link
Collaborator

Choose a reason for hiding this comment

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

It is not intended to be preserved over time 🫠

Copy link
Contributor

@LitoMore LitoMore Mar 6, 2025

Choose a reason for hiding this comment

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

BTW, why don't we change this command to view mode? We can list all supported converted values in a list. And we can always move the last used selection to the top for users.

In the meantime, we can change the input value to accept any string and use the color-string (I'm the maintainer of the color-string project as well) to detect user's input.

Comment on lines 19 to 24
if (props.arguments.text) {
const convertedColor = await getConvertedColor(props.arguments.text);
const convertedColor = await getConvertedColor(props.arguments.text, props.arguments.format);
if (convertedColor) {
await Clipboard.copy(convertedColor);
await showHUD("Copied color to clipboard");
}
Copy link
Contributor

Choose a reason for hiding this comment

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

logic: launchCommand should be wrapped in try/catch according to Raycast guidelines

Suggested change
if (props.arguments.text) {
const convertedColor = await getConvertedColor(props.arguments.text);
const convertedColor = await getConvertedColor(props.arguments.text, props.arguments.format);
if (convertedColor) {
await Clipboard.copy(convertedColor);
await showHUD("Copied color to clipboard");
}
try {
if (props.arguments.text) {
const convertedColor = await getConvertedColor(props.arguments.text, props.arguments.format);
if (convertedColor) {
await Clipboard.copy(convertedColor);
await showHUD("Copied color to clipboard");
}
}

@pernielsentikaer pernielsentikaer self-assigned this Mar 6, 2025
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
extension: color-picker Issues related to the color-picker extension extension fix / improvement Label for PRs with extension's fix improvements OP is contributor The OP of the PR is a contributor of the extension
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[color-picker] Can the conversion format be customized?
4 participants