-
Notifications
You must be signed in to change notification settings - Fork 5
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
feat: add new condition options and improve UX & UI #383
Conversation
37241b5
to
5ca7a05
Compare
73f0b79
to
dd15e55
Compare
@Soare-Robert-Daniel, here are my observations: For text conditions:
For number conditions:
The behavior is visible here - https://vertis.d.pr/v/A4wg2u For Select conditions: |
dd15e55
to
1d7fe35
Compare
Could not replicate.
I added auto-disable based on the field type on the target selector. Until now, it was based on operator value, but it is not a good UX. This should also fix the second problem.
I moved to their categories, but for backward compatibility reasons, they will be only available to basic select type input.
Could not replicate. It should be like this: |
Can't we just hide them, i.e we can use display none on those options? Is not clear if those are disabled since they are not available or not available in the plan. For PRO, once they are selected, instead of showing an empty select https://vertis.d.pr/i/PneRxH, we should display a CTA button with Upgrade to unlock. Any design can work. |
@Soare-Robert-Daniel, now, the Greater than and Less than appear under numerical type, but they cannot be selected: Also, when text is selected, the numerical conditions are still available: |
9ded19e
to
e694ba5
Compare
The new changes:
|
Thank you, @Soare-Robert-Daniel! Here are my observations:
|
018fc37
to
aed3d4f
Compare
3c0bdd3
to
95b04e1
Compare
Added Email and Date as supported fields. @selul @AndreeaCristinaRadacina Do you think that fields that are not supported should not be a part of the selector or should we show them as disabled (in the idea that it might be available in the future if they are asking for it) |
fields that dont supoort conditions should be removed from dropdown for now. |
ea81bfc
to
6382acc
Compare
Some important things to keep in mind. In the released version (store version), the conditions work correctly only for the inputs Thus, any new field and operator you see in this PR that is not mentioned above are new and not regressions. Not all fields are supported, and from the supported fields, not all operators are compatible (technical or UX reasons). New changes:
|
fb75cb3
to
44407da
Compare
44407da
to
fba815b
Compare
🎉 This PR is included in version 33.0.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Summary
Added new condition options for Free and Pro users. Improved the UI and UX.
Checklist:
between
operatorany
,empty
)contains
,not-contains
,regex
, etc )between
operatorImportant
The majority of the fields are not tracked by the conditions scripts. For them to work, we need to create more watchers tailored for each field.
Note
The code is very cumbersome, so much refactoring was done to simplify and better showcase the functionality.
Will affect visual aspect of the product
YES
Screenshots
Test instructions
Note
Not all fields are supported to be part of the conditions.
Check before Pull Request is ready:
Closes https://github.com/Codeinwp/ppom-pro/issues/429