Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Chart View v2 and other minor changes #1892

Merged
merged 3 commits into from
Jul 28, 2020
Merged

Chart View v2 and other minor changes #1892

merged 3 commits into from
Jul 28, 2020

Conversation

andresmgot
Copy link
Contributor

Description of the change

Adapted version of the ChartView:

Peek 2020-07-24 16-01

Other minor changes I fixed along the way:

  • I gathered all the icons import to the clarity wrapper so we don't need to add them in every component.
  • Added an icon to the AppList "Deploy" button, to match the one in the ChartView.
  • Refactored the "left-menu" style so it can be reused in different componetns.

Applicable issues

absoludity
absoludity approved these changes Jul 27, 2020
View reviewed changes
Copy link
Contributor

@absoludity absoludity left a comment

Choose a reason for hiding this comment

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

Not sure, but did you perhaps push the changes to the deploymentform here by mistake? Looks like a different PR. +1'd though.

dashboard/src/components/Catalog/ChartSummary.tsx Outdated
</h5>
<div>
{chartAttrs.sources.map((s, i) => (
<li key={i}>
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure if the old theme is also the same, but where is the parent ul or ol or ... here?

dashboard/src/components/ChartView/ChartView.v2.test.tsx Outdated
},
{
expected: false,
name: "random Helm repo",
Copy link
Contributor

Choose a reason for hiding this comment

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

Tiny nitpck: these sames could be more descriptive, eg, for this one: "a random Helm repo does not use github ids as names".

@andresmgot
Copy link
Contributor Author

Not sure, but did you perhaps push the changes to the deploymentform here by mistake? Looks like a different PR. +1'd though.

Yep, things that happen the last minute of a Friday :)

Andres Martinez Gotor added 2 commits July 27, 2020 11:00
@andresmgot andresmgot merged commit c8b5673 into master Jul 28, 2020
@andresmgot andresmgot deleted the chartView branch September 8, 2020 09:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@absoludity absoludity absoludity approved these changes

@Angelmmiguel Angelmmiguel Awaiting requested review from Angelmmiguel

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@andresmgot @absoludity