[KED-2981] Re-write Kedro UI buttons #716
Conversation
Signed-off-by: Rashida Kanchwala <[email protected]>
Signed-off-by: Rashida Kanchwala <[email protected]>
Signed-off-by: Rashida Kanchwala <[email protected]>
…org/kedro-viz into KED-2981/rewrite-kedro-ui-buttons
|
Just a quick comment off the top: I think we need to keep the secondary button style (and really any other styles that were used in Viz). For example, the edit runs modal has two button styles: Though with your change it looks like this: There should be some differentiation between those two. |
Signed-off-by: Rashida Kanchwala <[email protected]>
| @@ -181,7 +181,7 @@ const ExperimentWrapper = ({ theme }) => { | |||
| rel="noreferrer" | |||
| target="_blank" | |||
| > | |||
| <Button theme={theme}>View docs</Button> | |||
There was a problem hiding this comment.
I noticed you're removing the theme prop here, which is fine, but didn't remove it on the buttons in the src/components/export-modal/export-modal.js file. I wonder why? I think you can remove it there as well can't you?
There was a problem hiding this comment.
yes, i will remove it :) .. thanks!
Signed-off-by: Rashida Kanchwala <[email protected]>
src/components/button/button.js
Outdated
| `kui-button__btn--${size}`, | ||
| `kui-button__btn--${mode}` |
There was a problem hiding this comment.
Just curious... what's up with the ` instead of '?
There was a problem hiding this comment.
It's called template literals similar to f-string or % in python ( https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Template_literals)
src/components/button/button.scss
Outdated
| --color-button__text: #{rgba($color-light, 0.85)}; | ||
| } | ||
|
|
||
| .kui-button { |
There was a problem hiding this comment.
Please remove references of kui and rename all css classnames for this component to either kedro-button or just button ( also note that we are moving away from the pipeline prefix as well, which we have started doing from the experiment tracking components and will do so for new components moving forward)
There was a problem hiding this comment.
The code overall looks good! Requesting changes mainly for renaming the button classes as we are moving away from kui references. (I understand we have used kui-theme for the theme naming for the overall app given our previous setup and links to kui, which I have put in a ticket to rename those class name to strip away the kui reference( just theme--light or theme--dark, and can be done in one go after we have migrated all kui elements.)
Signed-off-by: Rashida Kanchwala <[email protected]>
Description
This PR is a part of the Kedro-UI migration ticket. In this PR we migrate the button functionality from Kedro-ui to Kedro-viz
Development notes
The Kedro-UI button functionality had lot of styles that we weren't using in Kedro-viz. So I have made that simple in this PR. In Kedro-UI, we have primary button, second button, small button, button with wipe transition. For consistency sake, I have used only one button style (primary) for all buttons on Kedro-viz. Additionally added the size small since it was used in the experiment tracking. I have removed secondary button style (which was also used for experiment tracking) and also removed the wipe transition (used in large pipeline warning) for the sake of consistency and simplicity.
UPDATE - As per the discussion with Tynan, and Gabriel in the check-in, I have added secondary mode for buttons (since it's used). I have removed wipe transition which was used in large pipelines - and Gabriel is comfortable with it.
I have also removed disable since we are not using it atm.
QA notes
Checklist
RELEASE.mdfile