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

Update date-converter extension #12456

Merged
merged 8 commits into from
May 26, 2024

Conversation

vineus
Copy link
Contributor

@vineus vineus commented May 19, 2024

Description

  • add 24h time format option (disabled by default)

Screencast

image

Raycast 2024-05-19 at 14 04 28

image

Raycast 2024-05-19 at 14 04 56

Checklist

- feat:add human readable time difference
- feat:add 24h format option
@raycastbot raycastbot added extension fix / improvement Label for PRs with extension's fix improvements extension: date-converter Issues related to the date-converter extension OP is contributor The OP of the PR is a contributor of the extension labels May 19, 2024
@raycastbot
Copy link
Collaborator

raycastbot commented May 19, 2024

Thank you for your contribution! 🎉

🔔 @asportnoy 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.

- changelog update
- feat:add human readable time difference
- changelog update
- feat:add human readable time difference
Copy link
Contributor

@asportnoy asportnoy left a comment

Choose a reason for hiding this comment

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

Could the relative info be part of the item description maybe? Also does it need to be that detailed?

@vineus
Copy link
Contributor Author

vineus commented May 19, 2024

@asportnoy

Could the relative info be part of the item description maybe? Also does it need to be that detailed?

Better?

date-converter 2024-05-19 at 22 50 48

- changelog update
- feat:add human readable time difference
- changelog update
- feat:add human readable time difference
@vineus vineus requested a review from asportnoy May 19, 2024 21:54
- changelog update
- feat:add human readable time difference
- changelog update
- feat:add human readable time difference
@asportnoy
Copy link
Contributor

  • I think the relative time is still too detailed. "9 minutes" should be plenty detailed for this, I think the seconds is overkill
  • Can you add a prefix or suffix like "in/ago"? So "9 minutes ago"
  • Separate the two parts with something like a hyphen and ditch the "relative:"

Copy link
Contributor

@asportnoy asportnoy left a comment

Choose a reason for hiding this comment

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

Marking as reviewed again - see above

- Revert "feat:add human readable time difference"
- changelog update
- feat:add human readable time difference
- feat:add 24h format option
- changelog update
- feat:add human readable time difference
- feat:add 24h format option
@vineus
Copy link
Contributor Author

vineus commented May 20, 2024

Reverted the relative time work.

I'll use my fork, I guess my needs are very specific.

@pernielsentikaer
Copy link
Collaborator

We can make it a preferences to show seconds or not, I like the idea 🙂

@pernielsentikaer pernielsentikaer self-assigned this May 21, 2024
@vineus
Copy link
Contributor Author

vineus commented May 23, 2024

Found some time to update.

Here is a shorter version:

image image image

@vineus vineus requested a review from asportnoy May 23, 2024 18:52
@pernielsentikaer
Copy link
Collaborator

@asportnoy could you check it once again

Copy link
Contributor

@asportnoy asportnoy left a comment

Choose a reason for hiding this comment

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

Much better! One minor thing and then I think it's good to go.

extensions/date-converter/src/index.tsx Outdated Show resolved Hide resolved
Co-authored-by: Albert Portnoy <albert@albertp.dev>
@vineus vineus requested a review from asportnoy May 24, 2024 14:54
Copy link
Contributor

@asportnoy asportnoy left a comment

Choose a reason for hiding this comment

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

Thanks for those changes, looks good to me.

@pernielsentikaer ready to merge when you are.

Copy link
Collaborator

@pernielsentikaer pernielsentikaer left a comment

Choose a reason for hiding this comment

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

Hi 👋

Looks good to me, approved 🔥

@pernielsentikaer pernielsentikaer merged commit c995b13 into raycast:main May 26, 2024
6 checks passed
Copy link
Contributor

Published to the Raycast Store:
https://raycast.com/asportnoy/date-converter

@raycastbot
Copy link
Collaborator

🎉 🎉 🎉

We've rewarded your Raycast account with some credits. You will soon be able to exchange them for some swag.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
extension: date-converter Issues related to the date-converter 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.

4 participants