RI-7687: adjust RDI deploy modal #5170
Conversation
ArtemHoruzhenko
requested review from
DimoHG,
KrumTy,
dantovska,
pawelangelow,
pd-redis and
valkirilov
as code owners
Code Coverage - Frontend unit tests
Test suite run success5266 tests passing in 688 suites. Report generated by 🧪jest coverage report action from a05694b |
| }} | ||
| button={ | ||
| <> | ||
| <Modal.Compose> |
There was a problem hiding this comment.
Tip: Do we need to compose the modal on our own, we already have a FormDialog that is a wrapper for the modal. If there is nothing custom here, we can go for it.
There was a problem hiding this comment.
well, I didn;t find a way to add data test id to rhe default buttons
There was a problem hiding this comment.
Ouch, that hurts. I'll give you my approval, but composing the whole modal just for the sake of providing two test IDs looks too much to me.
I would rather update the tests to use the labels of the buttons as a selector, instead of composing the modal and detaching it from Redis UI this way.
...ce/components/header/components/buttons/deploy-pipeline-button/DeployPipelineButton.spec.tsx
Show resolved
Hide resolved
...nstance/components/header/components/buttons/deploy-pipeline-button/DeployPipelineButton.tsx
Outdated
Show resolved
Hide resolved
...nstance/components/header/components/buttons/deploy-pipeline-button/DeployPipelineButton.tsx
Outdated
Show resolved
Hide resolved
| import { RiIcon } from 'uiSrc/components/base/icons/RiIcon' | ||
| import styles from './styles.module.scss' | ||
| import { RiTooltip } from 'uiSrc/components/base' | ||
| import { Modal } from '@redis-ui/components' |
There was a problem hiding this comment.
| import { Modal } from '@redis-ui/components' | |
| import { Modal } from 'uiSrc/components/base/display/modal' |
| > | ||
| Deploy | ||
| </PrimaryButton> | ||
| <> |
There was a problem hiding this comment.
| <> |
we can directly return the <Modal ...
pawelangelow
left a comment
pawelangelow
left a comment
There was a problem hiding this comment.
left a couple of comments about small issues to be fixed
ArtemHoruzhenko
deleted the
fe/feature/RI-7687-adjust-rdi-deploy-modal
branch
What
Moved frompopover to modal for deploy button
Testing