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

Show Package name when displaying PackageCard #38

Merged
merged 4 commits into from
Apr 7, 2024

Conversation

DylanLacey
Copy link
Contributor

Display each package's .name property on PackageCard, in small, [] wrapped text, just after the title. This helps users determine which packages they've already installed or used.

Resolves #35.

Display each package's `.name` property on `PackageCard`, in small, `[]` wrapped text, just after the title. This helps users determine which packages they've already installed or used.

Resolves espanso#35.
@arabello
Copy link
Collaborator

arabello commented Feb 20, 2024

Thanks for the PR @DylanLacey! Showing the name property is a good choice. What if we try something more stylish? For example we can use the extra free space on the top-right card corner instead of embracing it with [] and maybe use an ad-hoc component (eg. code with minimal appearance) to get more user attention.

What do you think?

Screenshot 2024-02-20 at 16 04 35

The Package name can be displayed in a way that's a bit more stylish, and should be displayed in multiple places.

Moved the package name from `PackageCard.tsx` to its own component, `PackageNamer.tsx`. Applied `code` styling and `green600` because it's a nice accent.
@DylanLacey
Copy link
Contributor Author

Great suggestion @arabello! I've made those changes.

Screenshot 2024-02-21 at 5 29 55 pm

I also think it's worth making the package name more obvious on the package details screen. Yes, it's present in the install command, but I think it also belongs with the other package metadata. I added it between the package title and author because the lower amount of horizontal space made placing it inline with the title look... weird. I've included those changes; See below.

Screenshot 2024-02-21 at 5 31 26 pm

Copy link
Collaborator

@arabello arabello left a comment

Choose a reason for hiding this comment

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

It looks nice!

Just one thing: the package.name was already shown as header title. If we want the package.name to be shown underneath the title using PackageNamer, we should change the header title to render the package.title. Otherwise we get a data duplication

Since the name is now displayed in a `code` tag under the title, as with the `PackageCard`, we should show the Package's above, instead.

(Also removed unused `Small` import)
@DylanLacey
Copy link
Contributor Author

GAH. Great point; I got mixed up because all the packages I was testing with had the same title as they did name >.>.

I've pushed those changes.

@AucaCoyan
Copy link
Member

Hello! Is this PR good to be merged? Thanks!
(BTW, I don't have write permissions, it needs a helping hand from @arabello 🙏🏼)

@arabello
Copy link
Collaborator

arabello commented Apr 7, 2024

Hey guys, sorry I'm used to the "who creates, merge" policy. I'm merging. @AucaCoyan I'm giving you writing permission

@arabello arabello merged commit 432ac99 into espanso:main Apr 7, 2024
1 check passed
@AucaCoyan
Copy link
Member

Thank you Matteo! and congratz to @DylanLacey for the merge! 🎉

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.

Display package name in search results
3 participants