Skip to content

Introducing UI changes to make configuration screen purpose clear #422

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Mar 7, 2017
Merged

Introducing UI changes to make configuration screen purpose clear #422

merged 3 commits into from
Mar 7, 2017
+17 −4

Conversation

@WojcikMike
Copy link
Contributor

@WojcikMike WojcikMike commented Mar 3, 2017

fixes #392

Adding labels to make it clear what toggle means. @sergioc please have a look from a UI perspective

@sergioc
Copy link
Contributor

sergioc commented Mar 3, 2017

@WojcikMike I'm not sure it adds much benefit. I think a more effective solution is to just rename the view's tab name from "Endpoints" to "Endpoints monitoring". That'll help infer that the purpose of the toggles is to turn monitoring ON or OFF.

@WojcikMike
Copy link
Contributor Author

WojcikMike commented Mar 3, 2017

I think that renaming the page is a step in the right direction, but I don't think it solves the problem. What if we would also added tooltip to the toggle?

@sergioc
Tweaks
c1d1044 
- tweaked list header copy
- tweaked list header style
- tweaked configuration screen tabs copy
- tweaked title of configuration pages
@sergioc
Copy link
Contributor

sergioc commented Mar 3, 2017

I didn't mean renaming the page, only the tab label. Anyway, adding those column headers will at most add some redundancy, so I can live with that without testing.

I've updated the PR.

@WojcikMike WojcikMike changed the title [WIP] Adding column description to configuration screen Adding column description to configuration screen Mar 6, 2017
@WojcikMike
Copy link
Contributor Author

WojcikMike commented Mar 6, 2017

LGTM to be merged. I tested it in chrome and IE and looks good.

@WojcikMike
Copy link
Contributor Author

WojcikMike commented Mar 6, 2017

@WilliamBZA can you have a look and merge if you are happy with it

@WojcikMike WojcikMike added this to the 1.8.3 milestone Mar 6, 2017
WilliamBZA
WilliamBZA reviewed Mar 7, 2017
View reviewed changes
src/ServicePulse.Host/app/js/directives/ui.particular.configurationTabs.tpl.html Outdated
<div class="col-sm-12">
<div class="tabs">
<h5 ng-class="{ active: isActive('/configuration/endpoints') }"><a href="#/configuration/endpoints">Endpoints ({{counters.endpoints | number}})</a></h5>
<h5 ng-class="{ active: isActive('/configuration/endpoints') }"><a href="#/configuration/endpoints">Endpoints monitoring ({{counters.endpoints | number}})</a></h5>
Copy link
Member

@WilliamBZA WilliamBZA Mar 7, 2017

Choose a reason for hiding this comment

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

Endpoints monitoring sounds strange, i feel like the tense is perhaps wrong.

What do you think about Monitored endpoints? @sergioc

Copy link
Contributor

@sergioc sergioc Mar 7, 2017

Choose a reason for hiding this comment

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

👍

Copy link
Contributor Author

@WojcikMike WojcikMike Mar 7, 2017

Choose a reason for hiding this comment

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

@WilliamBZA @sergioc change introduced

@WilliamBZA WilliamBZA merged commit cc7363e into master Mar 7, 2017
@WilliamBZA WilliamBZA deleted the improve_readability_of_configuration branch March 7, 2017 11:15
@WojcikMike WojcikMike changed the title Adding column description to configuration screen Introducing UI changes to make configuration screen purpose clear Mar 7, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@WilliamBZA WilliamBZA WilliamBZA approved these changes

+1 more reviewer

@sergioc sergioc sergioc left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

1.8.3

Development

Successfully merging this pull request may close these issues.

Configuration screen is unclear

4 participants

@WojcikMike @sergioc @WilliamBZA