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

remove node state link from client run page #404

Merged

Conversation

vinay033
Copy link
Collaborator

@vinay033 vinay033 commented May 22, 2019

Signed-off-by: vinay033 [email protected]

🔩 Description

During usability studies at ChefConf 2019 participants repeatedly clicked on the Node State link on the Client Runs page expecting something to happen. This task removes that link if we add more items to the left-nav of that page in the future we can add it back.

After talking with @jonong1972 this task is only removing the link from the Client Runs landing page, it will still be visible on node detail pages, we can revisit that in the future if necessary.

👍 Definition of Done

client_runs_page

⛓️ Related Resources

Fixes #397

Copy link
Contributor

@srenatus srenatus left a comment

Choose a reason for hiding this comment

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

🎉 thank you for that quick action

@@ -4,7 +4,7 @@
</div>

<app-server-org-filter-sidebar>
<chef-sidebar-entry route="/client-runs" icon="storage">Node State</chef-sidebar-entry>
<!-- <chef-sidebar-entry route="/client-runs" icon="storage">Node State</chef-sidebar-entry> -->
Copy link
Contributor

Choose a reason for hiding this comment

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

[nit] I'd rather just delete the line. git doesn't forget 😉

Copy link
Contributor

Choose a reason for hiding this comment

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

i think you might be able to pull out the whole nav too, and not just that one entry.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@srenatus I have removed the commented line. :)

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd merge, but I'm not sure I fully follow you there, @susanev: which whole nav would you like to see pulled?

Copy link
Contributor

Choose a reason for hiding this comment

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

yah, sorry i was not being clear. right now it renders a totally empty nav element when there are no items in the left-nav. i realize now, that workflow complicates things. the empty nav element can be annoying for people using screen readers. but its not important enough to fix in this pr i dont think. so ignore my comment, heh.

@vinay033 vinay033 force-pushed the Vinay/MSYS-1038_Remove_node_state_link_from_client_runs branch from 823f594 to 71dcc5c Compare May 23, 2019 06:58
@srenatus srenatus merged commit 62fb045 into master May 23, 2019
@chef-ci chef-ci deleted the Vinay/MSYS-1038_Remove_node_state_link_from_client_runs branch May 23, 2019 15:58
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.

Remove _Node State_ link from _Client Runs_
3 participants