-
Notifications
You must be signed in to change notification settings - Fork 4.5k
ui: Redesign - Instance Detail Proxy Info tab #7745
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: Redesign - Instance Detail Proxy Info tab #7745
Conversation
4bf79a7
to
3c840ea
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is mostly all good, typically I had a few comments that are more stylistic things, and there are a couple of things that we'd need to look at (mainly the namespace and prep queries things).
6e520c4
to
16e5b43
Compare
16e5b43
to
e6b751f
Compare
@johncowen I've updated Upstreams to include namespace design. Exposed Paths redesign will be in another PR. |
e6b751f
to
d8f7c3d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is all coming together nicely!
I left a few bits and pieces here. It would be good to get this merged though, let's catch up on it later and see if we can finish it off.
d8f7c3d
to
331fe30
Compare
<li class="port"> | ||
<CopyButtonFeedback | ||
@copy={{combinedAddress}} | ||
@name="Address to the clipboard" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool, I think its just this that needs making into "Address" otherwise the title
will say "Copy Address to the clipboard to the clipboard"
// copy-buttonfeed-back.hbs
(concat 'Copy ' name ' to the clipboard')
tbh I'm not even sure if that gets rendered anywhere, but if we did want to add 'to the clipboard' to the feedback tooltip that appears I'd say we should add that into the component itself, personally I wouldn't change that now tho.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, ok. Updating...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah cool, I think its just that one more tweak in the CopyButtonFeedback thing, and this is good to go! 👏
We noticed that it was about time we dug into the app-view component as the skin of the UI has chnged quite a bit since this component started and it has now become slightly more consistent. The %app-view CSS has not changed much and things needed within it have gradually become more clearer. Apart from some refactoring of CSS here, the biggest change is the addition of a `nav` Slot, which gets placed immediately underneath the title of the page. This contains any main navigation elements for the page.
331fe30
to
8dd005e
Compare
* Fix clickFirstAnchor bug * Create Proxy Info Tab for Instance Detail Page * Create tests for ProxyInfo and update other scenarios with Proxy data * ui: Refactors our app-view/%app-view component (#7752) Co-authored-by: John Cowen <[email protected]>
* Fix clickFirstAnchor bug * Create Proxy Info Tab for Instance Detail Page * Create tests for ProxyInfo and update other scenarios with Proxy data * ui: Refactors our app-view/%app-view component (#7752) Co-authored-by: John Cowen <[email protected]>
co-authored by: @johncowen