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

Advanced Settings to React/EUI #17465

Merged
merged 21 commits into from
May 7, 2018

Conversation

jen-huang
Copy link
Contributor

@jen-huang jen-huang commented Mar 29, 2018

Fixes #17045
Relates to #14791

This PR refactors the Management > Advanced Settings page to use React and EUI.

Table -> Form

Advanced Settings is now displayed as a page-long form, instead of a table. This was done after seeing the direction of the designs for Kibana v7.

screen shot 2018-03-29 at 2 37 41 pm

Categories

The initial page is extremely long, separated by Categories. The search bar can be used to filter settings like before, but there is now also a Category filter - this should hopefully reduce the user's need to scroll forever to find a setting. This paves work for v7 as well, though v7 will likely use submenu navigation for categories rather than filtering. Categorization is done by adding a new property category to each setting. Settings without a category are treated as general for purposes of this display.

Categories are listed alphabetically, except for General, which floats to the top.

mar-29-2018 14-43-30

Editing

Since all of the settings are now displayed as inline forms, there is no need to click an Edit button. Just start changing the value, and Save/Cancel actions will display next to the field. All field types support Escape to Cancel and most fields support Enter to Save (textarea and image fields do not).

mar-29-2018 14-52-32

JSON settings are validated. Settings can be cleared (reset back to default value) by clicking Reset below the field.

mar-29-2018 15-04-11

If the setting is an image and there isn't an image set, the file picker will display. If an image exists, it will be displayed with a Change link option below it. Image fields validate on max size, if there is one.

mar-29-2018 15-13-33

@jen-huang
Copy link
Contributor Author

jen-huang commented Mar 29, 2018

Here is how the current settings are categorized, as well as their user friendly names. Suggestions/Feedback on the categories as well as the actual categorization of settings highly wanted!!!

general:
    csv:quoteValues -> Quote CSV values
    csv:separator -> CSV separator
    dateFormat -> Date format
    dateFormat:dow -> Day of week
    dateFormat:scaled -> Scaled date format
    dateFormat:tz -> Timezone for date formatting
    defaultIndex -> Default index
    fields:popularLimit -> Popular fields limit
    filterEditor:suggestValues -> Filter editor suggest values
    filters:pinnedByDefault -> Pin filters by default
    format:bytes:defaultPattern -> Bytes format
    format:currency:defaultPattern -> Currency format
    format:defaultTypeMap -> Field type format name
    format:number:defaultLocale -> Formatting locale
    format:number:defaultPattern -> Number format
    format:percent:defaultPattern -> Percent format
    histogram:barTarget -> Target bars
    histogram:maxBars -> Maximum bars
    history:limit -> History limit
    indexPattern:fieldMapping:lookBack -> Recent matching patterns
    indexPattern:placeholder -> Index pattern placeholder
    indexPatterns:warnAboutUnsupportedTimePatterns -> Time pattern warning
    metaFields -> Meta fields
    metrics:max_buckets -> Maximum buckets
    query:allowLeadingWildcards -> Allow leading wildcards in query
    query:queryString:options -> Query string options
    savedObjects:listingLimit -> Objects listing limit
    savedObjects:perPage -> Objects per page
    search:queryLanguage -> Query language
    shortDots:enable -> Shorten fields
    sort:options -> Sort options
    state:storeInSessionStorage -> Store URLs in session storage
    telemetry:optIn -> Telemetry opt-in
    timepicker:quickRanges -> Time picker quick ranges
    timepicker:refreshIntervalDefaults -> Time picker refresh interval
    timepicker:timeDefaults -> Time picker defaults
    truncate:maxHeight -> Maximum table cell height
    xPack:defaultAdminEmail -> Admin email
dashboard:
    dashboard:defaultDarkTheme -> Dark theme
    xpackDashboardMode:roles -> Dashboards only roles
discover:
    context:defaultSize -> Context size
    context:step -> Context size step
    context:tieBreakerFields -> Tie breaker fields
    defaultColumns -> Default columns
    discover:aggs:terms:size -> Number of terms
    discover:sampleSize -> Number of rows
    discover:sort:defaultOrder -> Default sort direction
    doc_table:highlight -> Highlight results
notifications:
    notifications:banner -> Custom banner notification
    notifications:lifetime:banner -> Banner notification lifetime
    notifications:lifetime:error -> Error notification lifetime
    notifications:lifetime:info -> Info notification lifetime
    notifications:lifetime:warning -> Warning notification lifetime
reporting:
    xpackReporting:customPdfLogo -> PDF footer image
search:
    courier:customRequestPreference -> Custom request preference
    courier:ignoreFilterIfFieldNotInIndex -> Ignore filter(s)
    courier:maxSegmentCount -> Maximum segment count
    courier:setRequestPreference -> Request preference
timelion:
    timelion:default_columns -> Default columns
    timelion:default_rows -> Default rows
    timelion:es.default_index -> Default index
    timelion:es.timefield -> Time field
    timelion:graphite.url -> Graphite URL
    timelion:max_buckets -> Maximum buckets
    timelion:min_interval -> Minimum interval
    timelion:quandl.key -> Quandl key
    timelion:showTutorial -> Show tutorial
    timelion:target_buckets -> Target buckets
visualization:
    visualization:colorMapping -> Color mapping
    visualization:dimmingOpacity -> Dimming opacity
    visualization:loadingDelay -> Loading delay
    visualization:regionmap:showWarnings -> Show region map warning
    visualization:tileMap:WMSdefaults -> Default WMS properties
    visualization:tileMap:maxPrecision -> Maximum tile map precision
    visualize:enableLabs -> Enable labs

@snide
Copy link
Contributor

snide commented Mar 29, 2018

Oh man, this looks fantastic

@alexfrancoeur
Copy link

Woah! nice @jen-huang I'll pull down this week and play around with it. From your gif's, this looks fantastic

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@pickypg pickypg added the review label Mar 30, 2018
@skearns64
Copy link

This is great! 👍 👍

@timroes
Copy link
Contributor

timroes commented Mar 30, 2018

This is awesome! Thanks a lot ❤️

Just a few points I would like to discuss and hear your opinion on:

  1. I haven't had the time to test it out yet, so I just simply ask: do we have the possibility to link to individual settings, that the page is already filtered for them?
  2. I know this would cause some more work, but could we maybe besides the current id and description also add a "display name" (i.e. title, short name) for each setting? I would imagine, that this title would be shown above the element instead of the current id, and the id will just be visible is parentheses behind it, or via a tooltip. Some examples: CSV Separator (for csv:separator), Timezone (for dateFormat:tz), Start of week (for dateFormat:dow). That would make the settings even more user friendly in my opinion. Should be rather easy implementation wise, the hard step would be coming up with a name for all settings :-)

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@chrisronline
Copy link
Contributor

Pinging @elastic/kibana-visualizations @elastic/kibana-sharing @elastic/kibana-discovery to verify this separation makes sense to you

@chrisronline
Copy link
Contributor

This looks AWESOME!

I just a couple questions on the design:

There is quite a bit of dead space on the right. Maybe we can horizontally stack the areas if there is real estate?

screen shot 2018-03-30 at 9 46 56 am

It feels like the title should stand out more than it does. It's hard to separate the text visually when they are the same size/weight:
screen shot 2018-03-30 at 9 47 53 am

@nreese
Copy link
Contributor

nreese commented Mar 30, 2018

This looks really nice.

One comment about the categories is that some settings span more than one category. It would make sense to have the category be an array so settings can be part of multiple categories.

For example, courier:ignoreFilterIfFieldNotInIndex effects both visualize and dashboard.

@Bargs
Copy link
Contributor

Bargs commented Mar 30, 2018

The context options probably belong under Discover. I'd probably put all of the courier stuff under general, or maybe even under it's own category, like search or something. courier:ignoreFilterIfFieldNotInIndex for example applies everywhere, not just under Visualize, the same is true of most of those courier settings. Categorizing some of these settings is a tough call.

Looks like a really nice improvement, that screen was getting pretty crazy.

@thomasneirynck
Copy link
Contributor

wrt. categorization. imho this remains OK for visualizations.

@snide
Copy link
Contributor

snide commented Apr 2, 2018

Hey @jen-huang just some very minor designy suggestions. Feel free to take um or leave um.

To help give the form some room, I'd go ahead and use a max-width panel with a background to match the styling we have in EUI. Should help keep it from being jammed next to the left side.

image

On this dropdown, I'd probably keep the other sections around for selection, even if only one of them can be selected. Right now you have to clear a selection before you can select another one.

image

You might want to set your flexItems to grow={false}, then set the cancel button to be an empty button. Should make them a little less in your face and size appropriately.

image

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@elasticmachine
Copy link
Contributor

💔 Build Failed

@jen-huang
Copy link
Contributor Author

Thanks for the feedback, all! PR has been updated with the following changes:

  • search has been added as a category, containing all the courier settings. Moved context settings under discover. (@Bargs)
  • Changed category property to be an array, so that in the future when settings are broken into multiple category pages, we can display the same setting under each category it belongs to. Right now everything is still on one page and it doesn't make sense to display a setting multiple times, so each setting just has one entry in its category array. (@nreese)
  • Each setting now has a name property meant for displaying a user-friendly version of the setting key. List has been updated containing the names. (open to changes!) (@timroes)
  • Added deep-link route to a specific setting: #/management/kibana/settings/[key]. For example, #/management/kibana/settings/notifications:banner will display the page filtered down to the notifications:banner setting. (@timroes)
  • Updated UI with @snide's great suggestions. This improves the empty space on the right. Update to EUI 0.0.37 and add proper open sans fonts for EUI's K6 theme #17467 has also been merged which fixes bolding of the field labels. (@chrisronline)

UI changes with new friendly names:

screen shot 2018-04-02 at 1 36 56 pm

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@chrisronline
Copy link
Contributor

This is awesome!

When I filter down to a single category, then I reopen the selection list, I only see the one I previously selected which doesn't let me select multiple without unselecting, then reselecting once the entire list is repopulated.

screen shot 2018-04-03 at 11 07 08 am

Also, maybe we should be alternating background colors, or adding <hr> tags to separate rows because it's not immediately clear to me in this screenshot what I changed or am saving:

screen shot 2018-04-03 at 11 08 40 am

@jen-huang
Copy link
Contributor Author

@chrisronline
Thanks for catching the filter dropdown bug - fixed now!

I played around with some options for differentiating between field rows. It's really only an issue for the boolean settings, since there is a large gap between the EuiSwitch toggle and the Save button. The other types of settings are fine since their input fields take up the full 400px. Alternating colors / hr seemed too jarring when I tried, so instead I decreased the gutter spacing next to Save, and increased spacing between each setting:

screen shot 2018-04-03 at 2 11 55 pm

Hopefully this makes the distinction a little clearer. It's not perfect, but since the Save/Cancel buttons pop up immediately when editing, I'm not too concerned about the user not knowing what changed.

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Copy link
Contributor

@chrisronline chrisronline left a comment

Choose a reason for hiding this comment

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

LGTM!

@nreese
Copy link
Contributor

nreese commented Apr 9, 2018

I am concerned about the confusion caused by having 2 different search bar implementations. Kibana already has kuery for the filter bar. Kuery has a different syntax than EuiSearchBar. For example EuiSearchBar uses - to negate terms while kuery uses not. Is there a way to reconcile the search grammers between the two?

Sorry, wrong PR, ignore comment

@alexfrancoeur
Copy link

@jen-huang I wanted to take a look to provide some feedback but am running into some issues pulling it down on a fresh version of kibana. Any reason I'd be getting these errors?

From git://github.com/jen-huang/kibana
 * branch                  eui/advanced-settings -> FETCH_HEAD
Auto-merging yarn.lock
CONFLICT (content): Merge conflict in yarn.lock
Auto-merging src/core_plugins/timelion/index.js
Removing src/core_plugins/kibana/public/management/sections/settings/lib/get_editor_type.js
Auto-merging src/core_plugins/kibana/public/management/sections/settings/lib/__tests__/to_editable_config.test.js
Auto-merging src/core_plugins/kibana/public/management/sections/settings/lib/__tests__/get_val_type.test.js
Removing src/core_plugins/kibana/public/management/sections/settings/lib/__tests__/get_editor_type.js
Auto-merging src/core_plugins/kibana/public/management/sections/settings/lib/__tests__/get_aria_name.test.js
Removing src/core_plugins/kibana/public/management/sections/settings/advanced_row.js
Removing src/core_plugins/kibana/public/management/sections/settings/advanced_row.html
Auto-merging package.json
CONFLICT (content): Merge conflict in package.json
Automatic merge failed; fix conflicts and then commit the result.

@jen-huang
Copy link
Contributor Author

@alexfrancoeur I updated branch with master, that should resolve the dependency conflicts. You'll also need the corresponding x-pack-kibana branch if running x-pack: https://github.com/elastic/x-pack-kibana/pull/5089.

@jen-huang jen-huang force-pushed the eui/advanced-settings branch from 7b7f16c to b35be1b Compare April 25, 2018 19:38
@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@jen-huang
Copy link
Contributor Author

UI updated:

  • Categories separated out to different panels
  • Each setting uses new EuiDescribedFormGroup component (human title and description to the left of field)
  • Textarea fields replaced with code editor

screen shot 2018-05-07 at 12 41 55 pm

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.