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

Consul 1.1.0 UI Key/value list order #4116

Closed
kae opened this issue May 12, 2018 · 15 comments
Closed

Consul 1.1.0 UI Key/value list order #4116

kae opened this issue May 12, 2018 · 15 comments
Labels
theme/ui Anything related to the UI type/bug Feature does not function as expected

Comments

@kae
Copy link

kae commented May 12, 2018

Old UI or New UI
New UI

Describe the problem you're having

Unpredictable sort order of elements
capture

Describe the solution you'd like

Predictable order (based on key names, case insensitivity would be nice)
capture

Share inspiration

Yes

Also would like to give general feedback about new interface.
It's rather nice, but I'd like full-page experience for key/value tab (not a html table with fixed height and width).
Textarea control not only for json field.

And IMHO capability to edit value at same page with navigation was quite nice in old UI. Would be much happier if you reconsider your current UI for key/values.

Also implementation of filter interface (like in ACL tab) would be really convinient.

@kae
Copy link
Author

kae commented May 12, 2018

Similar problems with ACL list

In old version list was ordered based on id of acl.
Maybe in new version the same is true, but now id is not visible, so order looks like just random.

Maybe it would be nice to sort this list based on names too.

@scalp42
Copy link
Contributor

scalp42 commented May 14, 2018

Agreed here, for the visible IDs I opened: #4124

@kae
Copy link
Author

kae commented May 19, 2018

@opihana
Is there any reason that this issue has no feedback?
Can I add something more specific or do anything to improve content?

@johncowen johncowen added the theme/ui Anything related to the UI label May 21, 2018
@johncowen
Copy link
Contributor

Hi @kae thanks very much for the feedback! Really appreciate you trying out the beta for us, so please keep any more suggestions coming!

Apologies for the delay in coming back to you. We'll be working through issues/suggestions in batches as they come in and discussing with the team. We'll try and come back to you when there is any more news on this.

Thanks again!

@hannahhearth
Copy link
Contributor

@kae As John said, thanks so much for this feedback.

• predictable sorting of KVs sounds 👍
• full-page width sounds 👍
• Textarea control sounds 👍
• Editing directly from the list view -- Many folks said that important fields were too easy to edit--to accidentally make changes. So we added this step between viewing and editing across all list views in the UI.
• Filtering for KVs -- One follow-up question: What type of filtering would be most helpful: search by name or value, or filter by type (folder vs. key), or lock sessions?

@kae
Copy link
Author

kae commented May 22, 2018

@opihana

So we added this step between viewing and editing across all list views in the UI.

It's acceptable, just unaccustomed

One follow-up question: What type of filtering would be most helpful: search by name or value, or filter by type (folder vs. key), or lock sessions?

Personally I'd like to have filtering by name. Just as it works now in ACL (in old and new UI).
Everything else maybe useful for people, but if you will implement it, I think it should be some "advanced" control, not shown by default.

johncowen added a commit that referenced this issue Jun 1, 2018
@johncowen
Copy link
Contributor

Hi @kae,

We've been trying to replicate your main KV ordering issue in the new UI, but we can't replicate it right now. We are thinking there might be something that we aren't doing, or is different from what you have that means we can't replicate this.

I'm sure you've noticed that we are now ordering by 'folder' first, and then by 'file', but even when we have a good amount of KV's, for us, this ordering is correct ('folders' alphabetically, then 'files' alphabetically).

We'd really like to try and get to the bottom of this one and try to figure out what is happening and replicate before applying a fix. So if there is anything you can think of or suggest to help us figure out what might be different, that'd be great. As an additional thought, would it be possible for you to check the raw JSON API response you are getting when listing KV's to see what order consul is giving you?

Thanks,

@kae
Copy link
Author

kae commented Jun 4, 2018

@johncowen
/kv?keys&separator=%2F

Response (from Chrome debugger Network tab)

[
    "__secrets/",
    "auctionCatalogue-svc/",
    "auctionIntegrations-svc/",
    "auctionNotifications-svc/",
    "auctionSearch-svc/",
    "bp-svc/",
    "configurator/",
    "contentStore-svc/",
    "ctaRepository-import/",
    "ctaRepository-job/",
    "ctaRepository-svc/",
    "fias-svc/",
    "logs-svc/",
    "rmq-svc/",
    "rmqUtils/",
    "schedule-svc/",
    "vehicleAppraisal-svc/",
    "vehicleCatalogue-svc/",
    "vehicleTaxonomy-svc/"
]

UI (broken order)

image

So it's looks like issue is in frontend code
Browser is Chrome 67.0.3396.62 (Official Build) (64-bit) on windows (Russian locale used in system)

@johncowen
Copy link
Contributor

Great thanks @kae will come back asap, probably tomorrow now

@johncowen
Copy link
Contributor

Hi @kae ,

Thanks for passing over the JSON response, we've managed to reproduce the issue and understand what the issue is, and it's an interesting one!

We added 'sort by folder first' functionality to the new UI, previously in the old UI we just sorted by order that the Consul API gave us. Adding this functionality means we've started using javascript's sort for for KV listings, which is where things get interesting.

Basically, Chrome seems to be the only browser that doesn't always use a stable sorting algorithm. To make things more confusing, Chrome will use a stable sorting algorithm when your list is 10 items or less (or so it seems), which is why it was a little more tricky that usual to reproduce.

Finally, as we've been able to reproduce the issue now, thanks to your help, and understand the issue. We've applied a fix here that will likely be PR'ed later today.

Thanks again for reporting it to us, please keep any more coming! 👍

johncowen added a commit that referenced this issue Jun 7, 2018
Explicitly sort by Key after folder/file
@johncowen johncowen added the type/bug Feature does not function as expected label Jun 8, 2018
@johncowen
Copy link
Contributor

Hi @kae,

Just checking back in to see if you managed to try the UI in 1.2.0 yet, and to see if it's addressed your main issue of Key/Value ordering here? Please let me know if not.

If it has, then I'll be closing this issue over the next few days (or feel free to close it yourself).

There are a few other 'sub issues' in your main issue that we are tracking in other issues, but feel free to add separate issues for anything else specifically if you don't see it covered elsewhere.

Thanks again for your help with this!

@johncowen johncowen added the waiting-reply Waiting on response from Original Poster or another individual in the thread label Jul 3, 2018
@kae
Copy link
Author

kae commented Jul 3, 2018

@johncowen
Yes. Major issues was fixed. Thanks.

If there is some issues that tracks 'fixed layout with table' i'd like to follow it!

@johncowen
Copy link
Contributor

I think there are a couple that are related to that:

#4231 and #4230

We are currently discussing new layouts within the entire UI to make use of more screen real estate. Overall, we are looking at a more responsive layout for larger as well as smaller sized screens.

When you say 'fixed layout' is this what you mean? Ah, thinking that might be a typo, did you mean 'fixed width'? If so then yes, we are definitely looking at this, can't find a specific exact issue on GH though, feel free to make one if you want and we can keep you updated there.

Thanks,

@kae
Copy link
Author

kae commented Jul 3, 2018

@johncowen
Yes, I mean 'fixed layout' as in 'fixed width layout'

Will wait for it!

@johncowen
Copy link
Contributor

Hi @kae

Just a quick heads up that we've been working on a fullscreen layout in #4435 if you wanted to take a look at the grabs. It's part 1 of a larger piece of work, so there is more to come on that one.

Thanks

@johncowen johncowen removed the waiting-reply Waiting on response from Original Poster or another individual in the thread label Aug 7, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
theme/ui Anything related to the UI type/bug Feature does not function as expected
Projects
None yet
Development

No branches or pull requests

4 participants