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

Ensure unique url for service.resource-details, update all instances #900

Conversation

AllenBW
Copy link
Member

@AllenBW AllenBW commented Aug 31, 2017

In support of a task in this pt: https://www.pivotaltracker.com/story/show/147640893

No UX? I don't know what counts anymore, seems to me to be just a simple url change

TL;DR services.resource-details now has url of /services/:serviceId/resource=:vmId (used to be /services/:vmId)

This was getting terribly annoying, refreshing the resource details page only to have this happen:
screen shot 2017-08-31 at 10 21 03 am

Cuz our resource details url isn't unique 😭

Before: can you tell which one is a service details vs a resource details?

screen shot 2017-08-31 at 9 59 06 am

screen shot 2017-08-31 at 9 59 16 am

After: unique! also doesn't mind being reloaded or navigated to directly (via url)

screen shot 2017-08-31 at 10 22 52 am

@AllenBW AllenBW added this to the Sprint 68 Ending Sep 4, 2017 milestone Aug 31, 2017
@miq-bot
Copy link
Member

miq-bot commented Aug 31, 2017

Checked commit AllenBW@bb43ead with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
0 files checked, 0 offenses detected
Everything looks fine. 🏆

@@ -6,7 +6,7 @@ export function VmsDetailsState (routerHelper) {
function getStates () {
return {
'services.resource-details': {
url: '/:vmId',
url: '/:serviceId/resource=:vmId',
params: { viewType: null },
template: '<vm-details>',
title: N_('Resource Details')
Copy link
Contributor

Choose a reason for hiding this comment

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

Not related to this PR but I have been slowly replacing N_ with __ wherever I find them.

Copy link
Member Author

Choose a reason for hiding this comment

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

OOOOOO gooooood call 👍

Copy link
Contributor

@chalettu chalettu left a comment

Choose a reason for hiding this comment

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

changes look solid

@AllenBW
Copy link
Member Author

AllenBW commented Sep 5, 2017

@chriskacerguis 👀 🙇‍♀️

@chriskacerguis chriskacerguis merged commit 1855682 into ManageIQ:master Sep 5, 2017
@AllenBW AllenBW deleted the enhancement/ensure-unique-url-for-resource-details-state branch September 5, 2017 13:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants