Skip to content
This repository has been archived by the owner on Feb 23, 2024. It is now read-only.

Make Dropdown code reusable and reuse it among filters #7875

Closed
kmanijak opened this issue Dec 7, 2022 · 4 comments
Closed

Make Dropdown code reusable and reuse it among filters #7875

kmanijak opened this issue Dec 7, 2022 · 4 comments
Labels
block: filter by attribute Issues related to the Filter by Attribute block. block: filter by rating Issues related to the Filter by Rating block. block: filter by stock Issues related to the Filter by Stock block. type: cooldown Things that are queued for a cooldown period (assists with planning). type: technical debt This issue/PR represents/solves the technical debt of the project.

Comments

@kmanijak
Copy link
Contributor

kmanijak commented Dec 7, 2022

Description

There's couple of filters supporting dropdown version:

All of them have pretty similar implementation and there's quite a lot of duplicated code, mechanisms or styles.

Goal

The goal of this issue is to extract common logic/styles (whatever applicable) and reuse it among blocks to reuse the code and decrease the bundle size.

@kmanijak kmanijak added block: filter by price Issues related to the Filter by Price block. block: filter by rating Issues related to the Filter by Rating block. block: filter by attribute Issues related to the Filter by Attribute block. block: filter by stock Issues related to the Filter by Stock block. type: cooldown Things that are queued for a cooldown period (assists with planning). type: technical debt This issue/PR represents/solves the technical debt of the project. and removed block: filter by price Issues related to the Filter by Price block. labels Dec 7, 2022
@kmanijak kmanijak changed the title Make Dropdown code reusable and reuse among filters Make Dropdown code reusable and reuse it among filters Dec 8, 2022
@Aljullu Aljullu added the type: good first issue The issue is a good candidate for the first community contribution/for a newcomer to the team. label Dec 30, 2022
@roykho roykho self-assigned this Feb 2, 2023
@roykho roykho removed the type: good first issue The issue is a good candidate for the first community contribution/for a newcomer to the team. label Feb 2, 2023
@roykho
Copy link
Member

roykho commented Feb 16, 2023

So after taking a look at this, I am not sure if it warrants an extraction. There are indeed some repeated items however you would still need to pass many props such as remountKey, placeholder, displayTransform, messages and even styles if they're different between filter blocks.

If we did this, it would just mean we've replaced calling a component with another and not much gain. Unless I missed something, wouldn't mind discussing this further.

@kmanijak
Copy link
Contributor Author

Even though I was the one who created the issue, looking at this now, I must agree with you - it wouldn't add much value, while it could be time-consuming.

Maybe (just a maybe) it would make more sense to abstract "Filter" mechanism, since there's a lot of in common between:

  • Filter by Attribute
  • Filter by Rating
  • Filter by Stock
  • Filter by Price (although this one differs from the rest).
    But it will be pretty time-consuming as well and with a pretty complicated Filter by Attribute's logic it can be really troublesome to make a proper abstraction.
    The gain would be: simplified code-base (the logic there is very complicated and can be simplified for sure), a bit smaller bundle-size (Filters are currently one of the biggest blocks in the WooCommerce blocks, comparable with Cart and Checkout).

To summarise:

  • I agree with your point that it's pointless to abstract Dropdown logic
  • I think we could repurpose issue to abstract Filters in general which would have good impact (especially would improve the DX), but it's time-consuming, so I'm not sure if we should invest time in it.

@roykho roykho removed their assignment Feb 17, 2023
@roykho
Copy link
Member

roykho commented Feb 17, 2023

abstract Filters in general

Yeah I think that makes more sense.

@kmanijak
Copy link
Contributor Author

There's a dedicated project covering the Frontend Filters. Closing this one.

@kmanijak kmanijak closed this as not planned Won't fix, can't repro, duplicate, stale Nov 27, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
block: filter by attribute Issues related to the Filter by Attribute block. block: filter by rating Issues related to the Filter by Rating block. block: filter by stock Issues related to the Filter by Stock block. type: cooldown Things that are queued for a cooldown period (assists with planning). type: technical debt This issue/PR represents/solves the technical debt of the project.
Projects
None yet
Development

No branches or pull requests

3 participants