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 bring extension #14838

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

Update bring extension #14838

wants to merge 7 commits into from

Conversation

xilopaint
Copy link
Contributor

@xilopaint xilopaint commented Oct 8, 2024

Description

  • Updated the rendering logic of grid items to show non-added items only when the user enters a search term.
  • Added items are now grouped by sections.
  • Changed icons for the grid items to better represent the command actions for adding and removing items.
  • Changed the action panel title from the extension name to the list name.
  • Introduced an empty view for when the shopping list is empty.

Screenshot

bring-1
bring-2
bring-3

Checklist

- Update bring extension
- Initial commit
@raycastbot raycastbot added extension fix / improvement Label for PRs with extension's fix improvements extension: bring Issues related to the bring extension OP is contributor The OP of the PR is a contributor of the extension labels Oct 8, 2024
@raycastbot
Copy link
Collaborator

raycastbot commented Oct 8, 2024

Thank you for your contribution! 🎉

🔔 @amuelli @maximilianzuern 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.

You can expect an initial review within five business days.

@pernielsentikaer
Copy link
Collaborator

It looks good to me 🔥

@amuelli do you want to check this?

@pernielsentikaer pernielsentikaer self-assigned this Oct 9, 2024
@maximilianzuern
Copy link
Contributor

@xilopaint Thanks for your PR! I really like the improvements you've made.

One minor thing: what was the reason for removing the colored dot in ItemsGrid.tsx? I think using red and green is more in line with the bring! app and more user-friendly than the grey plus/minus.

@xilopaint
Copy link
Contributor Author

xilopaint commented Oct 9, 2024

@xilopaint Thanks for your PR! I really like the improvements you've made.

One minor thing: what was the reason for removing the colored dot in ItemsGrid.tsx? I think using red and green is more in line with the bring! app and more user-friendly than the grey plus/minus.

Hey, thanks for asking! There are a few reasons for the choice.

The Bring! app UI does not use colored dots but rather grids with red and green backgrounds for added and non-added items. The reason it uses different colors is that both added and non-added items are presented together. That's not the case here, as in the current implementation, added and non-added items are displayed separately. We only need to highlight the difference between the actions for adding and removing items, and for this matter, a circle with a plus sign and a circle with a minus sign are much more effective and universally recognizable for adding and removing actions than green and red solid dots with no signs. Some might argue we could still use colors for Icon.PlusCircleFilled and Icon.MinusCircleFilled anyway. However, the issue is that the item images lack colors. Using colors only on the icons would break design consistency and distract from the more important elements without providing any additional benefit. I believe the proposed visual aligns much better with the Raycast design concept.

@amuelli
Copy link
Contributor

amuelli commented Oct 9, 2024

@xilopaint First of all, many thanks for your continuing contributions. It's very nice to see others also taking an interest in this little extension.
It would be nice to understand the motivation behind all these changes. Whether something is an improvement or not can be sometimes very subjective.

Updated the rendering logic of grid items to show non-added items only when the user enters a search term.

I can see how this makes it a bit cleaner when looking at already added items. I somehow liked the discoverability of the whole cataloge without any search term. This might be subjective and I'm generally fine with that change overall.

Added items are now grouped by sections.

In the mobile app, this is configurable in the list order menu. I personally prefer it ungrouped as it displays more items in a condensed way, and I don't get much value from the section names. It would actually be nice to have both options and make it configurable in the extension, or possibly even better, base it on the setting from the app. The setting is actually exposed through the API with userSettings.purchaseStyle.

Changed icons for the grid items to better represent the command actions for adding and removing items.

I share the same concerns as @maximilianzuern. My initial implementation aimed to stay as close as possible to the design language of the Bring! app itself, with the assumption that any user of the extension is probably already familiar with the app and should immediately understand the meaning of the colors. I would have actually recreated the tiles with colored backgrounds if that had been possible in a reasonable manner. The colored circles were a compromise.
You do have a point that this is potentially at odds with the Raycast design language, but personally, in the context of Bring!, I would weigh the familiarity with the app higher.

The reason it uses different colors is that both added and non-added items are presented together. That's not the case here, as in the current implementation, added and non-added items are displayed separately.

Thanks for sharing your motivation behind it. However, I don't fully agree with the statement above. When searching for items, the results might also contain already added items. Here, I think it's important to easily distinguish them from non-added items, and I believe the color-coded circles make it much easier than the plain plus and minus icons that look almost the same at a glance. The alternative of excluding already added items from the results could potentially also be quite confusing as it diverges from how the Bring! app behaves.
If you are still convinced that the plus/minus icons are generally an improvement, I'm totally fine with introducing this as a configuration option for the extension.

@xilopaint
Copy link
Contributor Author

This might be subjective and I'm generally fine with that change overall.

Does it mean we can go with the new rendering logic?

or possibly even better, base it on the setting from the app. The setting is actually exposed through the API with userSettings.purchaseStyle.

I have implemented this suggestion.

When searching for items, the results might also contain already added items.

In my implementation you won't see already added items if you type anything in the search bar.

If you are still convinced that the plus/minus icons are generally an improvement, I'm totally fine with introducing this as a configuration option for the extension.

I'm convinced that the plus and minus signs are an improvement. They are universally recognized symbols for adding and removing actions and the primary color is consistent with the overall design. Additionally, the primary action titles, "Add to List" and "Remove from List," are always visible in the lower bar, so I don't see any room for confusion. I agree that using colored tiles would be ideal, but as you mentioned, it's not feasible with the current Raycast API. Using colors only for these icons would draw too much attention to a secondary UI element and distract from the main elements, which are the item icons. It would also break design consistency.

That said, I find it odd to introduce an option with different styles for this icons. There's a troubling trend in the extension ecosystem of adding options simply because people disagree. However, if you prefer, I can color the plus and minus icons green and red or revert to the original solid-colored circles. Just let me know your decision.

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

5 participants