-
Notifications
You must be signed in to change notification settings - Fork 414
RI-7681: [NEW UI] Connection Settings #5127
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
Conversation
50aa856 to
a1c37d7
Compare
e1c8b04 to
28bb562
Compare
redisinsight/ui/src/components/form-dialog/FormDialog.styles.ts
Outdated
Show resolved
Hide resolved
redisinsight/ui/src/pages/home/components/form/DatabaseForm.tsx
Outdated
Show resolved
Hide resolved
redisinsight/ui/src/pages/home/components/form/DatabaseForm.tsx
Outdated
Show resolved
Hide resolved
| className="form__divider" | ||
| /> | ||
|
|
||
| <Spacer size="m" /> |
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.
I am not sure about this pattern. It seems like there are a lot of cases, where some margin around the divider can be useful. Maybe it is worth it to explore adding props to Divider component to do this with styles?
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.
Well, sometimes we want space just before the divider, sometimes only after, sometimes there is space in some other component around it and it adds additional space. I agree that it is starting to look like a pattern but is either adding custom styling with margin everywhere, or the spacer IMO - adding to the divider will just make the component more complex.
...es/home/components/manual-connection/manual-connection-form/forms/EditSentinelConnection.tsx
Outdated
Show resolved
Hide resolved
c07b272 to
ff95d79
Compare
…roperty instead from dist
Code Coverage - Frontend unit tests
Test suite run success5229 tests passing in 681 suites. Report generated by 🧪jest coverage report action from 9591d7a |
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.
lgtm
Add changes to the Connection Settings when adding a database
Applied changes