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

[GlobalStep] User is able to set negative values in "Filter Products by Price" block. #2695

Closed
gglobalstep opened this issue Jun 11, 2020 · 11 comments · Fixed by #5123
Closed
Assignees
Labels
block: filter by price Issues related to the Filter by Price block. type: bug The issue/PR concerns a confirmed bug. type: good first issue The issue is a good candidate for the first community contribution/for a newcomer to the team.

Comments

@gglobalstep
Copy link

gglobalstep commented Jun 11, 2020

Environment:
Windows 10: Chrome(v83.0.4103.61)(64-bit), Firefox(v77.0.1)(64-bit)

Steps to Reproduce:

  1. Open a test site.
  2. Login with valid credentials.
  3. From dashboard go to Pages.
  4. Add new page.
  5. Add "All Products" block.
  6. Add "Filter Products by Price" block
  7. Add "Active Product filters" block
  8. Click on Preview or Publish the page.
  9. Go to the page having all the above block added.
  10. Enter min price 10 and max price 100 in filter.
  11. Now change max price to 0.
  12. Observe that negative values are displayed in min price field.

Note: Also All products show random products irrespective of the filter applied.

Actual Result:
User is able to set negative price in "Filter by Price" block.

Expected Result:
User should not be able to set negative price as product price cannot be negative.

Refer Artifact:
#2695.zip

@nerrad nerrad changed the title [GlobalStep - WooCommerce Blocks 2.7] User is able to set negative values in "Filter Products by Price" block. [GlobalStep] User is able to set negative values in "Filter Products by Price" block. Jun 11, 2020
@nerrad
Copy link
Contributor

nerrad commented Jun 11, 2020

Thanks for reporting this! I can reproduce.

This isn't a block bug so it has a lower priority but definitely something that will get added to our backlog.

@nerrad nerrad added type: bug The issue/PR concerns a confirmed bug. block: filter by price Issues related to the Filter by Price block. type: good first issue The issue is a good candidate for the first community contribution/for a newcomer to the team. labels Jun 11, 2020
@github-actions
Copy link
Contributor

This issue has been marked as stale because it has not seen any activity within the past 60 days. Remove the stale label or post a comment, otherwise it will be closed in 10 days.

@github-actions github-actions bot added the status: stale Stale issues and PRs have had no updates for 60 days. label Dec 12, 2020
@nerrad
Copy link
Contributor

nerrad commented Dec 15, 2020

Can reproduce still.

@nerrad nerrad removed the status: stale Stale issues and PRs have had no updates for 60 days. label Dec 15, 2020
@ralucaStan ralucaStan self-assigned this Jan 26, 2021
@ralucaStan
Copy link
Contributor

I think we need to define what the expected functionality for the text inputs is.

Thinks I have observed during testing:

  1. If the user selects the text in an input and writes - the input disappears
  2. Invalid values result in random outputs. I started with the selection below:

Screenshot 2021-01-29 at 14 52 19

  • writing positive values resets the interval, with different outputs

    • when I write for Max 20 (< Min). A new interval appears on the bar and in the inputs: Min: 10, Max: 20

    • Screenshot 2021-01-29 at 14 58 10
    • when I write for Max 10 (< Min). A new interval appears on the bar and in the inputs: Min: 0, Max: 10

    • Screenshot 2021-01-29 at 14 58 52
  • writing negative values resets the interval with negative values

    • I select 40 for Min and 90 for Max with the mouse. I then write for Max -80 (< Min). A new interval appears on the bar
    • Screenshot 2021-01-29 at 14 52 32

In the current setup, where input values are applied on focus (no button), one proposal could be:

  • make sure the user can't write Max values that are lower than the current Min value
  • make sure the user can't write Min values that are above the current Max value
  • make sure the user can't write negative values

@mikejolley
Copy link
Member

🐛 🐛 🐛 🐛

@ralucaStan regarding your proposal:

make sure the user can't write Max values that are lower than the current Min value

If we are currently filtering by min and max, say I am filtering from 20-40, and then I change my mind and want to filter between 0 and 20, if I try to edit max first it will fail. I would have to do min first, then max.

Do you think this is acceptable?

Would an alternative be to reset min to 0 if possible, before rejecting the new value?

make sure the user can't write Min values that are above the current Max value

Same as above. Do we reject, or shift max to the actual maximum or next interval above the inputted value?

make sure the user can't write negative values

👍🏻

We should also define what the expected result is from an invalid input. Do we:

  • Leave the value in the input but style it, for example, with a red border?
  • Ignore the input and revert to the current valid value?

@ralucaStan
Copy link
Contributor

If we are currently filtering by min and max, say I am filtering from 20-40, and then I change my mind and want to filter between 0 and 20, if I try to edit max first it will fail. I would have to do min first, then max.

For this use case, I think a button would be necessary because you are trying to modify the interval as a whole and not just a limit value. And that interval would also need to be validated on button click.

I was proposing solutions for modifying one of the limits of the existing interval. If the value from the input is invalid, then we could just revert to the previous value.

These are 2 possible use cases of the component, and I think we might need some help from @SpacePol to make this a usable component.

I also noticed that the component also has an option to show a button, but it does not influence the inputs' behavior. When the user writes new values, the slider is updated automatically. A button click is necessary to filter the products list.

Screenshot 2021-01-29 at 16 40 49

Screenshot 2021-01-29 at 16 39 57

@ghost
Copy link

ghost commented Feb 4, 2021

After reading the post, I think we can start by make a couple of things:

As @ralucaStan said, we should expect these behaviours:

  • make sure the user can't write Max values that are lower than the current Min value
  • make sure the user can't write Min values that are above the current Max value
  • make sure the user can't write negative values

I would add:

  • Obligate the user to just write Max values that are > 0 / Restrict the user to write Max values that are < 0.
  • Indicate an invalid input with an error message: "The value must be > 0"
  • Add a "tooltip-style" above each knob when moved to show the number that is being shown. This can help usability.
  • Add a "reset" button to quickly reset the operation.

When I try to use the filter, as soon as I change a value, the page reloads and goes up the top. We can change this behaviour and show an "Apply" button to trigger the operation. This will make the action expected.

@github-actions
Copy link
Contributor

github-actions bot commented Apr 5, 2021

This issue has been marked as stale because it has not seen any activity within the past 60 days. Our team uses this tool to help surface issues for review. If you are the author of the issue there's no need to comment as it will be looked at.

Internal: After 10 days with no activity this issue will be automatically be closed.

@github-actions github-actions bot added the status: stale Stale issues and PRs have had no updates for 60 days. label Apr 5, 2021
@nerrad nerrad removed the status: stale Stale issues and PRs have had no updates for 60 days. label Apr 7, 2021
@github-actions
Copy link
Contributor

github-actions bot commented Jun 7, 2021

This issue has been marked as stale because it has not seen any activity within the past 60 days. Our team uses this tool to help surface issues for review. If you are the author of the issue there's no need to comment as it will be looked at.

Internal: After 10 days with no activity this issue will be automatically be closed.

@github-actions github-actions bot added the status: stale Stale issues and PRs have had no updates for 60 days. label Jun 7, 2021
@nerrad nerrad removed the status: stale Stale issues and PRs have had no updates for 60 days. label Jun 18, 2021
@nerrad nerrad reopened this Jun 18, 2021
@github-actions
Copy link
Contributor

This issue has been marked as stale because it has not seen any activity within the past 60 days. Our team uses this tool to help surface issues for review. If you are the author of the issue there's no need to comment as it will be looked at.

Internal: After 10 days with no activity this issue will be automatically be closed.

@github-actions github-actions bot added the status: stale Stale issues and PRs have had no updates for 60 days. label Aug 18, 2021
@nerrad nerrad removed the status: stale Stale issues and PRs have had no updates for 60 days. label Aug 18, 2021
@github-actions
Copy link
Contributor

This issue has been marked as stale because it has not seen any activity within the past 60 days. Our team uses this tool to help surface issues for review. If you are the author of the issue there's no need to comment as it will be looked at.

Internal: After 10 days with no activity this issue will be automatically be closed.

@github-actions github-actions bot added the status: stale Stale issues and PRs have had no updates for 60 days. label Oct 18, 2021
@nerrad nerrad removed the status: stale Stale issues and PRs have had no updates for 60 days. label Oct 20, 2021
@gigitux gigitux self-assigned this Nov 9, 2021
gigitux added a commit that referenced this issue Nov 10, 2021
… Price block #2695

Don't allow to insert negative values on input for Filter Products By Price block
gigitux added a commit that referenced this issue Nov 10, 2021
… Price block #2695

Don't allow to insert negative values on input for Filter Products By Price block
gigitux added a commit that referenced this issue Jan 6, 2022
… on inputs (#5123)

* Don't allow to insert negative values on input for Filter Products By Price block #2695

Don't allow to insert negative values on input for Filter Products By Price block

* renaming util functions and add comments
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
block: filter by price Issues related to the Filter by Price block. type: bug The issue/PR concerns a confirmed bug. type: good first issue The issue is a good candidate for the first community contribution/for a newcomer to the team.
Projects
None yet
5 participants