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

Add handling for sorting by variant attributes #2378

Merged
merged 3 commits into from Jan 23, 2023

Conversation

cboelter
Copy link
Contributor

@cboelter cboelter commented Oct 7, 2022

As described here the sorting by variant attributes is not implemented yet: #2375
This pull request is a proposal to add sorting by variant attributes. I'm not really sure that this solves all cases, so it would be very nice to get some feedback :-)

@aschempp
Copy link
Member

Interesting approach. This only works if there is a default variant though, right?

@cboelter
Copy link
Contributor Author

You are right, I had the products without an default variant not in mind :-( ... I see there two approaches to handle them.

  1. Ignore Products without default variants and use the main product
  2. Get all variants and sort them first and use then the best matching variant for the sorting

Maybe you have more suggestions with your experience in isotope :-)

@aschempp aschempp added the bug label Jan 17, 2023
@aschempp aschempp added this to the 2.8.9 milestone Jan 17, 2023
@aschempp
Copy link
Member

I have updated the PR to simplify things. If there IS a default variant, we will use your new approach which should work fine. If there is none, we simply continue with the old version (basically using an empty value). Are you ok with that?

@cboelter
Copy link
Contributor Author

Yes, I'm absolutely okay with that. Thanks! :-)

@aschempp aschempp merged commit 5fede0f into isotope:2.8 Jan 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants