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

Move API key name to first column #895

Merged
merged 1 commit into from
Jul 2, 2019

Conversation

bafonins
Copy link
Contributor

@bafonins bafonins commented Jul 1, 2019

Summary

References #875

then maybe we should make the columns ID, name, rights.

Screenshot 2019-07-02 at 09 58 58

@bafonins bafonins added the c/console This is related to the Console label Jul 1, 2019
@bafonins bafonins added this to the July 2019 milestone Jul 1, 2019
@bafonins bafonins requested a review from kschiffer as a code owner July 1, 2019 10:30
@bafonins bafonins self-assigned this Jul 1, 2019
Copy link
Contributor

@htdvisser htdvisser left a comment

Choose a reason for hiding this comment

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

This may be a bit confusing. API key lists are currently sorted by their ID, so if the console now starts presenting this name as the first field, users will be confused if the list isn't sorted by that field, right?

@bafonins bafonins changed the title [Quickfix] Move API key name to first column Move API key name to first column Jul 1, 2019
@johanstokking
Copy link
Member

Yeah good point. If we keep the sorting by ID (makes sense, name is optional), then maybe we should make the columns ID, name, rights.

@bafonins
Copy link
Contributor Author

bafonins commented Jul 2, 2019

then maybe we should make the columns ID, name, rights.

How is this better than the order before?

@johanstokking
Copy link
Member

then maybe we should make the columns ID, name, rights.

How is this better than the order before?

The name was burried on the right

change order of the table columns
@bafonins bafonins force-pushed the feature/change-api-keys-table-headers branch from e1531e5 to 81c817a Compare July 2, 2019 07:59
@bafonins
Copy link
Contributor Author

bafonins commented Jul 2, 2019

updated description

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.02%) to 73.071% when pulling 81c817a on feature/change-api-keys-table-headers into ac50229 on master.

@kschiffer
Copy link
Member

The ID value is very unhelpful IMO. I don't see any user scanning the table by a cryptic hash value. If we can, we should really sort by creation date or the name value. We might even consider making the name field mandatory, or setting default values like API Key 1, which I would recommend. We can then hide or at least abbreviate the ID field in the table view (that is, if there is no essential use of the ID value that I have missed).

Copy link
Member

@kschiffer kschiffer left a comment

Choose a reason for hiding this comment

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

Approving with regards to: #875 (comment)

@bafonins bafonins merged commit 3d6f1e2 into master Jul 2, 2019
@bafonins bafonins deleted the feature/change-api-keys-table-headers branch July 2, 2019 11:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c/console This is related to the Console
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants