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: Redesign - Service Detail Page #7655

Merged
merged 8 commits into from
Apr 21, 2020

Conversation

kaxcode
Copy link
Contributor

@kaxcode kaxcode commented Apr 16, 2020

Preview Link

  • Create ConsulServiceInstanceList with styling and test
    • Implement ConsulServiceInstanceList to Service Detail page
    • Implement ConsulExternalSource to Services Show page header
    • Update services/show page object
  • Update the styling of CompositeRow
  • Refactor ConsulServiceList component and styling
  • Update ConsulExternalSource to say 'Registered via...'
  • Upgrade consul-api-double to patch 2.14.1
  • Fix up tests to not use Kind in service models

@kaxcode kaxcode requested a review from a team April 16, 2020 18:50
@kaxcode kaxcode added the theme/ui Anything related to the UI label Apr 16, 2020
@kaxcode kaxcode force-pushed the ui/feature/redesign-service-detail-page branch 2 times, most recently from 1b94371 to 9eaafc8 Compare April 16, 2020 22:28
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 hey!

Ok so looks like I said a lot here, don't let this fool you! I think the majority of the work here is all good, just a couple of nips and tucks, and I left a few comments for future work also.

Lemme know what you think, or we can catch up when you are around.

ui-v2/app/components/consul-service-list/index.hbs Outdated Show resolved Hide resolved
ui-v2/app/routes/dc/services/show.js Outdated Show resolved Hide resolved
ui-v2/app/styles/components/app-view.scss Outdated Show resolved Hide resolved
ui-v2/app/controllers/dc/services/show/instances.js Outdated Show resolved Hide resolved
ui-v2/app/styles/components/composite-row/skin.scss Outdated Show resolved Hide resolved
ui-v2/app/components/consul-service-list/index.hbs Outdated Show resolved Hide resolved
@kaxcode kaxcode force-pushed the ui/feature/redesign-service-detail-page branch 2 times, most recently from ed90a78 to eeff0d8 Compare April 20, 2020 18:35
@kaxcode kaxcode force-pushed the ui/feature/redesign-service-detail-page branch from eeff0d8 to 1cdc37b Compare April 20, 2020 20:52
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.

I left more notes for chatting through on things that aren't super important, so we can almost get this merged I think. I think the most important thing here before we do that is the null > model comment though.

P.S. Just noticed that I added some additional comments to some previously resolved convo's but github doesn't 'unresolve' them, not sure how I can do this, so be sure to open those convo's up down below here.

ui-v2/app/routes/dc/services/show.js Outdated Show resolved Hide resolved
ui-v2/app/routes/dc/services/show.js Outdated Show resolved Hide resolved
ui-v2/app/styles/core/typography.scss Show resolved Hide resolved
ui-v2/app/templates/dc/services/show.hbs Show resolved Hide resolved
ui-v2/app/templates/dc/services/show.hbs Show resolved Hide resolved
@kaxcode kaxcode force-pushed the ui/feature/redesign-service-detail-page branch from 1cdc37b to 153a61f Compare April 21, 2020 13:31
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! Thanks for spending the extra time to get this spot on, appreciated 👏

@kaxcode kaxcode merged commit a4857d0 into ui-staging Apr 21, 2020
@kaxcode kaxcode deleted the ui/feature/redesign-service-detail-page branch April 21, 2020 14:21
johncowen pushed a commit that referenced this pull request Apr 30, 2020
* Create ConsulServiceInstanceList with styling and test
* Implement ConsulServiceInstanceList to Service Detail page
* Implement ConsulExternalSource to Services Show page header
* Update services/show page object

* Update the styling of CompositeRow

* Refactor ConsulServiceList component and styling

* Update ConsulExternalSource to say 'Registered via...'

* Upgrade consul-api-double to patch 2.14.1

* Fix up tests to not use Kind in service models

* Update ListCollection with clickFirstAnchor action

* Add $typo-size-450 to typography base variables
kaxcode added a commit that referenced this pull request May 11, 2020
* Create ConsulServiceInstanceList with styling and test
* Implement ConsulServiceInstanceList to Service Detail page
* Implement ConsulExternalSource to Services Show page header
* Update services/show page object

* Update the styling of CompositeRow

* Refactor ConsulServiceList component and styling

* Update ConsulExternalSource to say 'Registered via...'

* Upgrade consul-api-double to patch 2.14.1

* Fix up tests to not use Kind in service models

* Update ListCollection with clickFirstAnchor action

* Add $typo-size-450 to typography base variables
johncowen pushed a commit that referenced this pull request May 12, 2020
* Create ConsulServiceInstanceList with styling and test
* Implement ConsulServiceInstanceList to Service Detail page
* Implement ConsulExternalSource to Services Show page header
* Update services/show page object

* Update the styling of CompositeRow

* Refactor ConsulServiceList component and styling

* Update ConsulExternalSource to say 'Registered via...'

* Upgrade consul-api-double to patch 2.14.1

* Fix up tests to not use Kind in service models

* Update ListCollection with clickFirstAnchor action

* Add $typo-size-450 to typography base variables
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