RI-7681: Apply new UI changes to DB Info section on Edit modal #5149
Conversation
dantovska
requested review from
ArtemHoruzhenko,
DimoHG,
KrumTy,
pawelangelow,
pd-redis and
valkirilov
as code owners
Code Coverage - Frontend unit tests
Test suite run success5260 tests passing in 687 suites. Report generated by 🧪jest coverage report action from ab46e31 |
| const { connectionType, nameFromProvider, sentinelMaster, host, port } = props | ||
|
|
||
| const handleCopy = (text = '') => { | ||
| navigator.clipboard.writeText(text) |
There was a problem hiding this comment.
We have similar functionality in so many places. We should extract it to utility function
There was a problem hiding this comment.
@valkirilov noted me of that, aren't you handling this already? If so, please share so I can follow what you are doing?
| export const DbInfoGroup = styled(Group)` | ||
| background-color: ${({ theme }) => | ||
| theme.semantic.color.background.neutral200}; | ||
| border: 1px solid ${({ theme }) => theme.semantic.color.border.neutral400}; |
There was a problem hiding this comment.
I think this is starting to get out of hand, we use semantic.color.border in 26 places (at least), 21 are neutral500, one of 400, 600, 800, and one primary600.
This is more like a question to everybody - do you think it makes sense to define the colors we use in RI explicitly? Like borderColor, bgColor etc ?
There was a problem hiding this comment.
I'm not sure keeping our version in sync with redis ui would be easy
There was a problem hiding this comment.
Yeah, for sure, it might come with some struggles (keeping them in sync), but also, I see Redis UI introduces major breaking changes, so we're also kind of blocked now (we can't get the latest bug fixes and features because of that).
And I think that soon we'll end up in a state where we don't treat it as a main source of truth, but instead build an intermediate abstract layer that depends on it. Just like we did with our components (RiIcon, RiTabs, etc.), we might need something similar for the color palette/theme.
For sure, it's not something straightforward (maybe), but we should definitely consider it and think more about it, and how we can handle it in an efficient way.
There was a problem hiding this comment.
@pd-redis I don't know if it will be expected to have the same border everywhere? And we will need an effort to change that. I don't just want to introduce the variable, without changing everywhere and then we will end up like the eui colors that we are trying to get rid off ... borderLight, borderLighter ... It does makes sense if we do a redesign again and want to change border everywhere... Anyway, I don't think it should start in this PR
| export const DbInfoGroup = styled(Group)` | ||
| background-color: ${({ theme }) => | ||
| theme.semantic.color.background.neutral200}; | ||
| border: 1px solid ${({ theme }) => theme.semantic.color.border.neutral400}; |
There was a problem hiding this comment.
I'm not sure keeping our version in sync with redis ui would be easy
redisinsight/ui/src/pages/home/components/form/sentinel/DbInfoSentinel.tsx
Outdated
Show resolved
Hide resolved
… the cluster db host port info
dantovska
force-pushed
the
fe/feature/RI-7681-edit-db-info
branch
from
7897fd3 to
8ab2785
Compare
…t theme, slightly darker in the dark theme.
What
Testing
Standalone
Cluster
Sentinel
Redis Stack