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

refactor: min max validation in filter slider #56

Open
wit03 opened this issue Oct 9, 2024 · 0 comments
Open

refactor: min max validation in filter slider #56

wit03 opened this issue Oct 9, 2024 · 0 comments

Comments

@wit03
Copy link
Member

wit03 commented Oct 9, 2024

          _:warning: Potential issue_

Fix validation logic in input onChange handlers.

The input fields for manual entry of min and max values are a good addition. However, there are a couple of issues in the validation logic:

  1. The error message for the minimum range is incorrect in both inputs. It should reference min, not max.
  2. In the minimum value input, when the value is below the allowed range, it's setting the maximum value instead of the minimum.

Please apply the following fixes:

For the minimum value input:

- toast.error("Min range is " + max)
+ toast.error("Min range is " + min)
- setMaxVal(Number(0))
+ setMinVal(Number(min))

For the maximum value input:

- toast.error("Min range is " + max)
+ toast.error("Min range is " + min)

Also, consider adding some spacing between the input fields and the "to" text for better readability:

- <p className="text-sm font-normal text-gray-700">to</p>            
+ <p className="text-sm font-normal text-gray-700 mx-2">to</p>            
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

                <div className="flex  lg:justify-around items-center gap-2 mt-4">
                    <div className="w-20">
                    <Input
                    type="text"
                    value={minVal}
                    onChange={(e:React.ChangeEvent<HTMLInputElement>) => {
                        if(Number(e.target.value) > max){
                        toast.error("Max range is " + max)
                        setMinVal(Number(max))
                        
                        }
                        else if(Number(e.target.value) < min){
                        setMinVal(Number(min))
                        toast.error("Min range is " + min)
                        }
                        else {
                        setMinVal(Number(e.target.value))
                        }
                    }}
                    className="w-full p-2 border border-gray-200 rounded bg-gray-50 text-sm font-normal text-gray-500 focus:ring focus:ring-primaryRed"
                    />
                    </div>
                  
                  <p className="text-sm font-normal text-gray-700 mx-2">to</p>            

                <div className="w-20">
                <Input
                    type="text"
                    value={maxVal}
                    onChange={(e:React.ChangeEvent<HTMLInputElement>) => {
                     if(Number(e.target.value) > max){
                        toast.error("Max range is " + max)
                        setMaxVal(Number(max))
                        
                      }
                      else if(Number(e.target.value) < min){
                        setMaxVal(Number(0))
                        toast.error("Min range is " + min)
                      }
                      else {
                        setMaxVal(Number(e.target.value))
                      }
                    }}
                    className="w-full p-2 border border-gray-200 rounded bg-gray-50 text-sm font-normal text-gray-500 focus:ring focus:ring-primaryRed"
                    />
                </div>
                </div>    

Originally posted by @coderabbitai[bot] in #46 (comment)

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

No branches or pull requests

1 participant