-
Notifications
You must be signed in to change notification settings - Fork 65
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
Update filter components to support non metacard based filters, add better number validation #966
Conversation
andrewkfiedler
commented
Jul 29, 2024
•
edited
Loading
edited
- It's useful to use these filter components on non metacard filters in order to build UI components that hit solr for other things, which this will allow.
- The number validation on some of the filters was preventing users from doing things like deleting their value and starting with a negative, so it's been rewritten to keep values around locally no matter what, and only propagate those as changes to the filter when they're valid. If unvalid, the user is notified that the previous valid value will be used instead.
An updated dist branch has been created and pushed to origin. Remember to use "yarn install --force" if you want to pick up changes each time you make a change to this branch by committing. |
An updated dist branch has been created and pushed to origin. Remember to use "yarn install --force" if you want to pick up changes each time you make a change to this branch by committing. |
64e53cf
to
9fe22d9
Compare
An updated dist branch has been created and pushed to origin. Remember to use "yarn install --force" if you want to pick up changes each time you make a change to this branch by committing. |
9fe22d9
to
c6e4909
Compare
…etter number validation - It's useful to use these filter components on non metacard filters in order to build UI components that hit solr for other things, which this will allow. - The number validation on some of the filters was preventing users from doing things like deleting their value and starting with a negative, so it's been rewritten to keep values around locally no matter what, and only propagate those as changes to the filter when they're valid. If unvalid, the user is notified that the previous valid value will be used instead.
c6e4909
to
c156b51
Compare
}} | ||
/> | ||
) | ||
user | ||
.get('user') | ||
.get('preferences') | ||
.set('dateTimeFormat', Common.getDateTimeFormats()['ISO']['minute']) | ||
setTimeout(() => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoids the issue where we generate an onChange before the expected onChange around precision.
@@ -59,6 +59,10 @@ const validateDate = ( | |||
} | |||
|
|||
export const DateAroundField = ({ value, onChange }: DateAroundProps) => { | |||
const validValue = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move the handling of optional / invalid values up to this common point rather than down in the component.
value: value.buffer.amount, | ||
} | ||
: {})} | ||
validation={(val) => val > 0} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Notice we can pass in extra validation to the number field, in this case to say we only allow positive numbers.
@@ -26,6 +26,10 @@ const isInvalid = ({ value }: Props) => { | |||
} | |||
|
|||
export const DateRelativeField = ({ value, onChange }: Props) => { | |||
const validValue = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar change to move handling of invalid values to the top of the component.
An updated dist branch has been created and pushed to origin. Remember to use "yarn install --force" if you want to pick up changes each time you make a change to this branch by committing. |
React.useState('') | ||
|
||
React.useEffect(() => { | ||
if ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here is where we decide if we will send the new value to the filter (or other parent component with an onChange).
An updated dist branch has been created and pushed to origin. Remember to use "yarn install --force" if you want to pick up changes each time you make a change to this branch by committing. |
@@ -0,0 +1,221 @@ | |||
import { FilterClass, ValueType, ValueTypes } from './filter.structure' | |||
|
|||
export const isRelativeValue = ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A bunch of utilities to make it possible to narrow down what type of filter you're handling.
) | ||
} | ||
|
||
export const isValueEmpty = (filter: FilterClass): boolean => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A general utility around deciding if a filter has been "filled out" so to speak and is valid.
@@ -149,3 +152,38 @@ export const getComparators = (attribute: string): ComparatorType[] => { | |||
} | |||
return comparators | |||
} | |||
|
|||
export const ComparatorContext = React.createContext({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is what enables us to properly assign comparators in non metacard situations.
build now |
Internal build has been started, your results will be available at build completion. |
Build SUCCESS See the job results in legacy Jenkins UI or in Blue Ocean UI. |
You can no longer use: Instead, if the pr was merged you can now access these changes by using: Remember to use "yarn install --force" if you want to pick up changes to this version. |