-
Notifications
You must be signed in to change notification settings - Fork 384
RI-7190 Field Box component #4716
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
RI-7190 Field Box component #4716
Conversation
FieldTypes, | ||
} from 'uiSrc/pages/browser/components/create-redisearch-index/constants' | ||
|
||
// TODO: Add colors mapping for tags when @redis-ui/components v38.6.0 is released |
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.
Badge
component in @redis-ui/components v38.6.0 supports custom color out of the box (docs), so we can colorize the different types of fields, once we update the dependency version.
Note: Currently @redis-ui/components
and @redis-ui/styles
. Unfortunately, the latter comes with some breaking changes in its latest version, so the upgrade process might need additional effort, and I won't do it as part of this task.
import { VectorSearchBox } from 'uiSrc/components/new-index/create-index-step/field-box/types' | ||
import { FieldTypes } from 'uiSrc/pages/browser/components/create-redisearch-index/constants' | ||
|
||
// TODO: Maybe transform this to a factory function, so it can be reused more easily |
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.
Using factories (like Fishery) makes it easier to generate consistent, realistic mock data in our tests without repeating the same setup code over and over. It helps keep the tests clean and focused on what they're actually testing - not on building all the data manually every time.
For example, instead of having this static object and reusing it everywhere, we can define a factory:
const userFactory = Factory.define(() => ({
id: '123',
name: 'Test User',
email: '[email protected]',
}));
And then just use:
// Create a new example object
const user = userFactory.build();
// Or create an array of example objects
const users = userfactory.buildList(3)
// We can also easily override fields when needed
const adminUser = userFactory.build({ role: 'admin' });
This keeps tests flexible, readable, and easier to maintain as our data model grows, because we have to update the interface in a single place in the code.
PS: It's just a proposal for a future enhancement, so we can discuss it, and I would be happy to hear thoughts or alternatives!
import { FieldTypes } from 'uiSrc/pages/browser/components/create-redisearch-index/constants' | ||
|
||
// TODO: Maybe transform this to a factory function, so it can be reused more easily | ||
// TODO: Maybe make the values more dynamic with faker, so we can test more cases |
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.
By combining Faker with our mocks/factories, we can easily generate realistic, randomized data for our test cases. This helps avoid overly hardcoded or repetitive test data and gives us more variety and confidence that our code works with different inputs - not just the same values every time.
For example:
import { Factory } from 'fishery';
import { faker } from '@faker-js/faker';
const userFactory = Factory.define(() => ({
id: faker.string.uuid(),
name: faker.person.fullName(),
email: faker.internet.email(),
}));
Now each test run gets slightly different (but still valid) data, which can help us catch edge cases or assumptions we might otherwise miss.
Of course, if we need consistent values for a specific test, we can always override them manually:
userFactory.build({ email: '[email protected]' });
So it’s a nice balance of flexibility and realism in our tests!
PS: It's just a proposal for a future enhancement, so we can discuss it, and I would be happy to hear thoughts or alternatives!
dde0494
to
0664a81
Compare
expect(buildNewIndexTabContent).toBeInTheDocument() | ||
}) | ||
|
||
it('shouldn\'t switch to "Build new index" tab when clicked, since it is disabled', () => { |
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.
very very very nit: maybe it's personal pref, but should not
or "shouldn't.."
is read better IMO
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.
No worries, I was under the impression we have forced preferences over '
.
Now the title of the test is fixed, as suggested.
@@ -0,0 +1,40 @@ | |||
import React from 'react' |
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.
Just checking - was CreateIndexStepWrapper
intentionally added to this PR?
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.
Good question - initially, I started the work on this component, based on the branch related to the Tabs
component, with the idea to add the Field Boxes
inside the "Use Preset Index" tab. Later on, I gave up on this idea and provided this PR, with the component being stand-alone.
So, for safety’s sake, I just rebased the branch with the latest state of feature/RI-6855/vector-search
, and I hope we're safe to move on with this one.
…vention re #RI-7091
re #RI-7190
* used to combine multiple fields boxes in a single group re #RI-7190
ee4dfbd
to
71a9d5a
Compare
Code Coverage - Frontend unit tests
Test suite run success4816 tests passing in 635 suites. Report generated by 🧪jest coverage report action from 71a9d5a |
Description
Create base components for the "field box" and a "group of field boxes" that can be used in the "Create Index" step part of the new Vector Search feature
FieldBox
andFieldBoxesGroup
, that can be used together to present the interface for picking which fields to be added to the new Vector Search indexHow to test it
These are base components that will be included in the new wizard for the Vector Search, but currently, you can't find them integrated anywhere in the flow so far. In the first version of the feature, they should look similar to the following preview
How to use it
FieldBox
componentFieldBoxesGroup
componentFull Example