-
Notifications
You must be signed in to change notification settings - Fork 2
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(search): Enable data-only checkbox #3744
Conversation
Removed vultr server and associated DNS entries |
7610459
to
a154cf7
Compare
a154cf7
to
b22d06d
Compare
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.
Quick questions after a first pass - will come back to this one for a proper review !!
const toggleDataOnly = () => | ||
isDataOnly() | ||
? formik.setFieldValue("facets", ALL_FACETS) | ||
: formik.setFieldValue("facets", DATA_FACETS); |
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.
The logic here feels backwards to me based on naming?
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.
Thanks, updated naming 👍
"data.schema.fields.data.description", | ||
"data.schema.fields.data.options.description", | ||
"data.schema.fields.data.options.text", | ||
"data.schema.fields.data.options.data.val", |
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.
nit: I think data.schema.fields.data.options.data.val
should be considered a data field only search term, rather than general search?
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.
Fantastic catch thank you! 🦅 👀 Looks like a copy/paste error
What does this PR do?
DATA_FACETS
orALL_FACETS
ALL_FACETS
Screen.Recording.2024-10-03.at.10.54.58.mov
Next steps...