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.

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

ui: Fix empty SVG height #10122

Merged
merged 5 commits into from
Apr 28, 2021
Merged

Conversation

kaxcode
Copy link
Contributor

@kaxcode kaxcode commented Apr 26, 2021

Adding a conditional that prevents the Topology view from breaking when there are no upstreams in the upstream column.

Before
Screen Shot 2021-04-27 at 9 57 52 AM

After
Screen Shot 2021-04-27 at 9 58 34 AM

@kaxcode kaxcode added theme/ui Anything related to the UI backport/1.10 labels Apr 26, 2021
@kaxcode kaxcode requested a review from johncowen April 26, 2021 15:19
@vercel vercel bot temporarily deployed to Preview – consul April 26, 2021 15:24 Inactive
@vercel vercel bot temporarily deployed to Preview – consul-ui-staging April 26, 2021 15:24 Inactive
Copy link
Contributor

@johncowen johncowen left a comment

Choose a reason for hiding this comment

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

Hey 👋!

I'm not 100% sure what this fixes, could you add something to the description for me?

Apart from that I left a little inline comment suggestion.

@kaxcode
Copy link
Contributor Author

kaxcode commented Apr 27, 2021

Hey 👋!

I'm not 100% sure what this fixes, could you add something to the description for me?

Apart from that I left a little inline comment suggestion.

@johncowen I've added a description.

@kaxcode kaxcode requested a review from johncowen April 27, 2021 13:59
@johncowen
Copy link
Contributor

Hey thanks for fixing this up. I was wondering if this is a good opportunity to add some additional testing around this somewhere. What do you think? We could test for 0 'streams in each column and make sure no errors are thrown?

@kaxcode
Copy link
Contributor Author

kaxcode commented Apr 27, 2021

Hey thanks for fixing this up. I was wondering if this is a good opportunity to add some additional testing around this somewhere. What do you think? We could test for 0 'streams in each column and make sure no errors are thrown?

@johncowen I'll have to leave this for GA. I'm going to be changing some things for that *(All Services) card.

Copy link
Contributor

@johncowen johncowen left a comment

Choose a reason for hiding this comment

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

LGTM! One little thing, I think when we spoke you mentioned changing the changelog a little, not sure if you are going to do that or not.

@vercel vercel bot temporarily deployed to Preview – consul April 28, 2021 13:20 Inactive
@vercel vercel bot temporarily deployed to Preview – consul-ui-staging April 28, 2021 13:20 Inactive
@kaxcode kaxcode merged commit ce98d08 into master Apr 28, 2021
@kaxcode kaxcode deleted the ui/bug/fix-empty-upstreams-column-svg-drawing branch April 28, 2021 13:22
mikemorris pushed a commit that referenced this pull request May 5, 2021
mikemorris pushed a commit that referenced this pull request May 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
theme/ui Anything related to the UI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants