Skip to content

adjusted with marginTop#2935

Merged
msivasubramaniaan merged 2 commits intoredhat-developer:mainfrom
msivasubramaniaan:2931-helm-chart-installation-modal-the-icon-name-and-install-button-should-line-up
Jun 5, 2023
Merged

adjusted with marginTop#2935
msivasubramaniaan merged 2 commits intoredhat-developer:mainfrom
msivasubramaniaan:2931-helm-chart-installation-modal-the-icon-name-and-install-button-should-line-up

Conversation

@msivasubramaniaan
Copy link
Copy Markdown
Collaborator

@msivasubramaniaan msivasubramaniaan commented Jun 5, 2023

Signed-off-by: msivasubramaniaan msivasub@redhat.com

Fix: #2931

Post Fix:
image

Signed-off-by: msivasubramaniaan <msivasub@redhat.com>
@codecov
Copy link
Copy Markdown

codecov bot commented Jun 5, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (76469c0) 37.32% compared to head (216eb05) 37.32%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2935   +/-   ##
=======================================
  Coverage   37.32%   37.32%           
=======================================
  Files          54       54           
  Lines        3652     3652           
  Branches      716      716           
=======================================
  Hits         1363     1363           
  Misses       2289     2289           

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

mohitsuman
mohitsuman previously approved these changes Jun 5, 2023
Copy link
Copy Markdown
Contributor

@mohitsuman mohitsuman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@datho7561
Copy link
Copy Markdown
Contributor

It's not in the centre of the box:

StillNotInCentreOfBox

Signed-off-by: msivasubramaniaan <msivasub@redhat.com>
@msivasubramaniaan
Copy link
Copy Markdown
Collaborator Author

image

Copy link
Copy Markdown
Contributor

@mohitsuman mohitsuman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

Copy link
Copy Markdown
Contributor

@datho7561 datho7561 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, but we should go back and clean up this code. We can avoid hardcoding margins by using modern css features like flexbox

className={this.props.cardItemStyle.cardImage}
style={{ margin: '0rem' }} />
<div style={{ margin: '0rem' }}>
<div style={{ marginTop: '0.6rem' }}>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are better ways to accomplish this than hardcoding the margin, such as flexbox. However, I think this is okay for the release.

@msivasubramaniaan msivasubramaniaan merged commit a5dd2b7 into redhat-developer:main Jun 5, 2023
@msivasubramaniaan msivasubramaniaan deleted the 2931-helm-chart-installation-modal-the-icon-name-and-install-button-should-line-up branch June 5, 2023 17:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Helm Chart installation modal: the icon, name and install button should line up

3 participants