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 for disabling users #17333

Merged
merged 40 commits into from Jun 25, 2019
Merged

UI for disabling users #17333

merged 40 commits into from Jun 25, 2019

Conversation

@alexanderzobnin
Copy link
Contributor

alexanderzobnin commented May 28, 2019

This is PR which adds UI for disabling users (part of #11188). It's based on the extended API from #17305.
This is a draft, so let's discuss how the UI should look like.

Screenshot 2019-05-28 at 20 28 27

@alexanderzobnin

This comment has been minimized.

Copy link
Contributor Author

alexanderzobnin commented May 31, 2019

Changes regarding discussion with @dprokop

Screenshot from 2019-05-31 10-50-44

Screenshot from 2019-05-31 10-51-03

@dprokop

This comment has been minimized.

Copy link
Member

dprokop commented May 31, 2019

Alex, I think the label should be dimmed as well. What we could do here is also move the delete button to the user view to keep all the actions together. Then the actions in that view should be somehow separated, as now the deactivate button looks like it belongs to Organisations section.

@dprokop

This comment has been minimized.

Copy link
Member

dprokop commented May 31, 2019

Feels like the dimmed row only might not be enough to indicate the user being not active. What about having two lists, one for active and one for non-active users?

@alexanderzobnin

This comment has been minimized.

Copy link
Contributor Author

alexanderzobnin commented May 31, 2019

@dprokop I can do list sorted by enabled/disabled flag on the backend side

@torkelo

This comment has been minimized.

Copy link
Member

torkelo commented Jun 8, 2019

yes, needs some text & icon or icon + tooltip to indicate disabled

@torkelo

This comment has been minimized.

Copy link
Member

torkelo commented Jun 13, 2019

Any progress on this PR?

@alexanderzobnin alexanderzobnin marked this pull request as ready for review Jun 20, 2019
@alexanderzobnin

This comment has been minimized.

Copy link
Contributor Author

alexanderzobnin commented Jun 20, 2019

Now it looks like

Screenshot from 2019-06-20 14-09-45

@torkelo could you explain this e little bit more?

needs some text & icon or icon + tooltip to indicate disabled

@alexanderzobnin

This comment has been minimized.

Copy link
Contributor Author

alexanderzobnin commented Jun 20, 2019

@torkelo @dprokop maybe we should put information about auth modules (LDAP, OAuth, etc) to the edit user page as well?

@torkelo

This comment has been minimized.

Copy link
Member

torkelo commented Jun 20, 2019

That looks really bad, a tooltip on hover for the button instead?

, is there a reason why you can’t disable external users? Seems like a useful thing to be able to do.

@torkelo

This comment has been minimized.

Copy link
Member

torkelo commented Jun 20, 2019

The spacing between form and buttons look wrong , are you using gf-form-group class?

@torkelo

This comment has been minimized.

Copy link
Member

torkelo commented Jun 20, 2019

Yes, more info on this page would be good , auth mode, last login etc. maybe the delete action as well

@alexanderzobnin

This comment has been minimized.

Copy link
Contributor Author

alexanderzobnin commented Jun 20, 2019

@torkelo

That looks really bad, a tooltip on hover for the button instead?

What do you mean? That's a tooltip on hover. Printscreen just removed mouse pointer.

is there a reason why you can’t disable external users? Seems like a useful thing to be able to do.

It's by design and there's API restriction as well:

// External users shouldn't be disabled from API

The reason is that external users are managed outside Grafana. If you have LDAP sync enabled, disabled user can be re-enabled on the next sync.

@alexanderzobnin

This comment has been minimized.

Copy link
Contributor Author

alexanderzobnin commented Jun 20, 2019

The spacing between form and buttons look wrong , are you using gf-form-group class?

This is gf-form-button-row inside gf-form-group

<form name="userForm" class="gf-form-group">
  <div class="gf-form-button-row">
     <button type="submit" class="btn btn-danger"
@torkelo

This comment has been minimized.

Copy link
Member

torkelo commented Jun 20, 2019

Sorry did not see that it was a tooltip , looked like text above the buttons

@alexanderzobnin

This comment has been minimized.

Copy link
Contributor Author

alexanderzobnin commented Jun 25, 2019

@torkelo should I work on this?

needs some text & icon or icon + tooltip to indicate disabled

alexanderzobnin and others added 6 commits Jun 25, 2019
@alexanderzobnin alexanderzobnin changed the base branch from get-ext-users to master Jun 25, 2019
Copy link
Member

torkelo left a comment

The edit user page needs a redesign but will do that after the reactify user profile / edit user PR is merged

@alexanderzobnin alexanderzobnin changed the title Initial disable user UI UI for disabling users Jun 25, 2019
@torkelo torkelo merged commit 77375f3 into master Jun 25, 2019
2 checks passed
2 checks passed
build-branches-and-prs Workflow: build-branches-and-prs
Details
license/cla Contributor License Agreement is signed.
Details
@torkelo torkelo deleted the disable-user-ui branch Jun 25, 2019
ryantxu added a commit to ryantxu/grafana that referenced this pull request Jun 25, 2019
* grafana/master:
  release: update latest.json to v6.2.5 (grafana#17767)
  release: 6.2.5 changelog (grafana#17766)
  Fix typo s/Applicaiton/Application/ in error messages (grafana#17765)
  UserAdmin: UI for disabling users (grafana#17333)
  API: get list of users with additional auth info (grafana#17305)
ryantxu added a commit to ryantxu/grafana that referenced this pull request Jun 25, 2019
* grafana/master:
  release: update latest.json to v6.2.5 (grafana#17767)
  release: 6.2.5 changelog (grafana#17766)
  Fix typo s/Applicaiton/Application/ in error messages (grafana#17765)
  UserAdmin: UI for disabling users (grafana#17333)
ryantxu added a commit to ryantxu/grafana that referenced this pull request Jun 25, 2019
* grafana/master: (64 commits)
  release: update latest.json to v6.2.5 (grafana#17767)
  release: 6.2.5 changelog (grafana#17766)
  Fix typo s/Applicaiton/Application/ in error messages (grafana#17765)
  UserAdmin: UI for disabling users (grafana#17333)
  API: get list of users with additional auth info (grafana#17305)
  TimePicker: fixed minor issues with new timepicker (grafana#17756)
  Explore: Parses and updates TimeSrv in one place in Explore (grafana#17677)
  @grafana/ui: release (grafana#17754)
  Password: Remove PasswordStrength (grafana#17750)
  Devenv:SAML: devenv block with saml test app (grafana#17733)
  LDAP:Docs: add information on LDAP sync feature and update LDAP sync default (grafana#17689)
  Graph: Add data links feature (click on graph) (grafana#17267)
  Explore: Changes LogsContainer from a PureComponent to a Component (grafana#17741)
  Chore: Remove tether and tether drop dependency in grafana/ui (grafana#17745)
  noImplicitAny: time region manager etc. (grafana#17729)
  Panel: Fully escape html in drilldown links (was only sanitized before)  (grafana#17731)
  Alerting: Improve alert rule testing (grafana#16286)
  Elasticsearch: Visualize logs in Explore (grafana#17605)
  Grafana-CLI: Wrapper for `grafana-cli` within RPM/DEB packages and config/homepath are now global flags (grafana#17695)
  Add guidelines for SQL date comparisons (grafana#17732)
  ...
shavonn added a commit that referenced this pull request Jun 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.