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

Revamp managing list items API for Twig #107

Closed
engram-design opened this issue Sep 17, 2022 · 1 comment
Closed

Revamp managing list items API for Twig #107

engram-design opened this issue Sep 17, 2022 · 1 comment

Comments

@engram-design
Copy link
Member

engram-design commented Sep 17, 2022

What are you trying to do?

Because this plugin was mainly ported from Shortlist for Craft 2 and Expression Engine, the list-management API is a bit gross.

{% set item = craft.wishlist.item(entry.id) %}

<a href="{{ item.addUrl() }}">Add to List</a>
<a href="{{ item.removeUrl() }}">Remove from List</a>
<a href="{{ item.toggleUrl() }}">Toggle</a>

{% if item.getInList() %}
    <a href="{{ item.removeUrl() }}">Remove from List</a>
{% else %}
    <a href="{{ item.addUrl() }}">Add to List</a>
{% endif %}

All these require first fetching a Wishlist Item element, then acting on that. It's a little bit of a weird concept to add an element to a list, by first getting the Wishlist Item element (which won't exist), or even when trying to check if it's in a list. To check if an entry is in a list, you really just need to list to check against, and the entry. Not trying to throw a Wishlist Item query into the mix.

This change will not only help with performance (we don't need to create elements for each item element), but will hopefully be clearer and more Craft-like.

What's your proposed solution?

We should change the APIs to be centred around lists. The above should be represented like:

Managing items

{# See below - shortcut for getting the user's own list #}
{% set list = craft.wishlist.getUserList() %}

{# While longer to write, is more direct. Generates a URL for adding an element to a list #}
<a href="{{ craft.wishlist.addUrl(list.id, entry.id) }}">Add to List</a>
<a href="{{ craft.wishlist.removeUrl(list.id, entry.id) }}">Remove from List</a>
<a href="{{ craft.wishlist.toggleUrl(list.id, entry.id) }}">Toggle</a>

{# Now performs checks on the element (not Wishlist Item element) being in the list #}
{% if list.getInList(entry.id) %}
    <a href="{{ craft.wishlist.removeUrl(list.id, entry.id) }}">Remove from List</a>
{% else %}
    <a href="{{ craft.wishlist.addUrl(list.id, entry.id) }}">Add to List</a>
{% endif %}

Getting the default user list

As an added bonus, it'd be great to add a shortcut for getting the default user's list, which is 50% of the time what people want (unless they have multiple or complicated lists).

{% set list = craft.wishlist.lists().default(true).one() %}
vs
{% set list = craft.wishlist.getUserList() %}
or
{% set list = craft.wishlist.getUserList({ type: 'collection', title: 'Cars' }) %}

You can also add properties to add to the list query, in case you want to fetch the user lists of a given type, title and more.

So this change will centre more about fetching the list first for what you want to act on, then using the Wishlist plugin-level functions to perform actions.

Alternative syntax

Another alternative to shorten things is using custom Twig functions.

<a href="{{ wishlistAddUrl(list.id, entry.id) }}">Add to List</a>
<a href="{{ wishlistRemoveUrl(list.id, entry.id) }}">Remove from List</a>
<a href="{{ wishlistToggleUrl(list.id, entry.id) }}">Toggle</a>

or

<a href="{{ addToWishlist(list.id, entry.id) }}">Add to List</a>
<a href="{{ removeFromWishlist(list.id, entry.id) }}">Remove from List</a>
<a href="{{ toggleInWishlist(list.id, entry.id) }}">Toggle</a>

But I'm not really sure the benefit they bring. I also prefer the consistency of add/remove/toggle + Url as opposed to the add to/remove from/toggle in which makes sense in English, but creates inconsistencies when writing (and may be difficult for non-English speakers).

Additional context

No response

@engram-design
Copy link
Member Author

Updated in 3.0.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant