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

First pass at applying EUI styles to the index pattern creation page #15722

Merged

Conversation

chrisronline
Copy link
Contributor

Relates to #15721

Applies EUI styles to the index pattern creation page.

Note, the styling is a little off, but I wanted to some work out and ready for review before attempting to polish.

Screenshots:
screen shot 2017-12-20 at 1 54 58 pm
screen shot 2017-12-20 at 1 55 25 pm
screen shot 2017-12-20 at 1 55 39 pm
screen shot 2017-12-20 at 1 55 44 pm
screen shot 2017-12-20 at 1 55 53 pm

Copy link
Contributor

@cjcenizal cjcenizal left a comment

Choose a reason for hiding this comment

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

This is already looking so much better! I found a few areas to polish. I also need to make some updates to EUI which will require a couple small changes to class names in this PR.

<div class="euiSpacer euiSpacer--m"></div>

<!-- Actions -->
<div class="euiFlexGroup euiFlexGroup--gutterLarge euiFlexGroup--justifyContentSpaceAround euiFlexGroup--responsive">
Copy link
Contributor

Choose a reason for hiding this comment

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

We can change lines 160-194 to:

  <!-- Actions -->
  <div class="euiFlexGroup euiFlexGroup--gutterSmall euiFlexGroup--justifyContentFlexEnd">
    <div class="euiFlexItem euiFlexItem--flexGrowZero">
      <button
        class="euiButtonEmpty euiButtonEmpty--primary kuiButton--iconText"
        ng-click="stepTimeField.goToPreviousStep()"
      >
        <span class="euiButtonEmpty__content">
          <svg aria-hidden="true" class="euiIcon euiIcon--medium" xmlns="http://www.w3.org/2000/svg"
            xlink="http://www.w3.org/1999/xlink" width="16" height="16" viewBox="0 0 16 16">
            <defs>
              <path id="arrow_left-a" d="M13.069 5.157L8.384 9.768a.546.546 0 0 1-.768 0L2.93 5.158a.552.552 0 0 0-.771 0 .53.53 0 0 0 0 .759l4.684 4.61c.641.631 1.672.63 2.312 0l4.684-4.61a.53.53 0 0 0 0-.76.552.552 0 0 0-.771 0z"
              />
            </defs>
            <use fill-rule="nonzero" transform="rotate(90 8 8)" href="#arrow_left-a"
            />
          </svg>
          <span>Back</span>
        </span>
      </button>
    </div>
    <div class="euiFlexItem euiFlexItem--flexGrowZero">
      <button
        data-test-subj="createIndexPatternCreateButton"
        ng-disabled="!stepTimeField.canCreateIndexPattern()"
        class="euiButton euiButton--primary"
        type="submit"
      >
        <span class="euiButton__content">
          Create index pattern
        </span>
      </button>
    </div>
  </div>

This will render like this:

image


<div class="euiSpacer euiSpacer--m"></div>

<button
Copy link
Contributor

Choose a reason for hiding this comment

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

I noticed a bug where clicking this once you have a time field selected will submit the form. We can add type="button" to fix this (without it the button defaults to behave as if type="submit"):

  <button
    class="euiButtonEmpty"
    type="button"
    data-id="toggleButton"
    ng-click="stepTimeField.toggleAdvancedOptions()"
  >

indices="stepIndexPattern.allIndices"
pattern="stepIndexPattern.indexPatternName"
>
<span class="euiTextColor euiTextColor--primary">
Copy link
Contributor

Choose a reason for hiding this comment

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

We're going to remove euiTextColor--primary from EUI because it looks too much like a link. So I think we can remove this wrapping span entirely.

Copy link
Contributor

Choose a reason for hiding this comment

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

We need to remove references to euiTextColor--primary because it's going to be removed from EUI.

aria-label="Please wait while we fetch your time field options"
aria-describedby="timeFilterFieldSelectLabel"
>
<svg aria-hidden="true" class="euiIcon euiIcon--medium" xmlns="http://www.w3.org/2000/svg"
Copy link
Contributor

Choose a reason for hiding this comment

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

TODO: Replace this with refresh icon once @cchaos has merged it into EUI.


<div class="euiSpacer euiSpacer--m"></div>

<div class="timeFieldNameLabel">
Copy link
Contributor

Choose a reason for hiding this comment

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

We should use euiFormRow whenever we have groups of labels, inputs, and help text like this. Here's how we can use this pattern for lines 18-95:

  <div class="euiFormRow">
    <div class="euiFormLabel">
      <div class="euiFlexGroup euiFlexGroup--justifyContentSpaceBetween">
        <div class="euiFlexItem euiFlexItem--flexGrowZero">
          <label
            id="timeFilterFieldSelectLabel"
            for="timeFilterFieldSelect"
          >
            Time Filter field name
          </label>
        </div>

        <div class="euiFlexItem euiFlexItem--flexGrowZero">
          <a
            ng-if="!stepTimeField.isFetchingTimeFieldOptions"
            class="euiLink euiText euiText--extraSmall"
            ng-click="stepTimeField.fetchTimeFieldOptions()"
            kbn-accessible-click
            aria-describedby="timeFilterFieldSelectLabel"
          >
            Refresh
          </a>

          <p
            ng-if="stepTimeField.isFetchingTimeFieldOptions"
            class="euiTextColor euiTextColor--subdued"
            aria-label="Please wait while we fetch your time field options"
            aria-describedby="timeFilterFieldSelectLabel"
          >
            <svg
              aria-hidden="true"
              class="euiIcon euiIcon--medium"
              xmlns="http://www.w3.org/2000/svg"
              width="16"
              height="16"
              viewBox="0 0 16 16"
            >
              <g fill="none" fill-rule="evenodd">
                <circle cx="5" cy="5" r="1" fill="#13252D" />
                <circle cx="10" cy="5" r="1" fill="#13252D" />
                <circle cx="7.5" cy="7.5" r="7" stroke="#13252D" />
                <path stroke="#13252D" stroke-linecap="round" stroke-linejoin="bevel"
                d="M3 8c1.167 2 2.667 3 4.5 3 1.833 0 3.333-1 4.5-3" />
              </g>
            </svg>
          </p>
        </div>
      </div>
    </div>

    <div class="euiFormControlLayout">
      <select
        ng-show="stepTimeField.canShowMainSelect()"
        id="timeFilterFieldSelect"
        aria-describedby="timeFilterFieldSelectHelpText1 timeFilterFieldSelectHelpText2"
        class="euiSelect euiSelect--large"
        data-test-subj="createIndexPatternTimeFieldSelect"
        ng-disabled="stepTimeField.isTimeFieldSelectDisabled()"
        ng-required="stepTimeField.hasTimeFieldOptions()"
        ng-options="option as option.display disable when option.isDisabled for option in stepTimeField.timeFieldOptions"
        ng-model="stepTimeField.selectedTimeFieldOption"
      ></select>

      <select
        ng-show="stepTimeField.canShowLoadingSelect()"
        id="timeFilterFieldSelect"
        aria-describedby="timeFilterFieldSelectHelpText1 timeFilterFieldSelectHelpText2"
        class="euiSelect euiSelect--large"
        data-test-subj="createIndexPatternTimeFieldSelect"
        disabled="disabled"
      >
        <option>Loading...</option>
      </select>

      <p
        ng-if="stepTimeField.canShowNoTimeBasedFieldsMessage()"
        class="euiTextColor euiTextColor--subdued euiText euiText--small"
      >
        The indices which match this index pattern don't contain any time fields.
      </p>
    </div>

    <div
      ng-if="stepTimeField.canShowHelpText()"
      class="euiFormHelpText euiFormRow__text"
      id="timeFilterFieldSelectHelpText1"
    >
      The Time Filter will use this field to filter your data by time.
    </div>

    <div
      ng-if="stepTimeField.canShowHelpText()"
      class="euiFormHelpText euiFormRow__text"
      id="timeFilterFieldSelectHelpText2"
    >
      You can choose not to have a time field, but you will not be able to narrow down your data by a time range.
    </div>
  </div>

This will render like this:

image

<!-- Index pattern id input -->
<div class="euiSpacer euiSpacer--m"></div>
<div ng-if="stepTimeField.showAdvancedOptions">
<label
Copy link
Contributor

Choose a reason for hiding this comment

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

We should use euiFormRow here too.

<div class="euiSpacer euiSpacer--xs"></div>
<div class="euiFlexGroup">
<div class="euiFlexItem">
<p
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's move these two paragraphs into the help text of the above euiFormRow.

Copy link
Contributor

Choose a reason for hiding this comment

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

Once we do that, we can wrap the form row and the action button in a euiFlexGroup to get the alignment right. Pseudo-code showing what I mean:

    <div class="euiFlexGroup euiFlexGroup--justifyContentSpaceBetween euiFlexGroup--alignItemsEnd euiFlexGroup--responsive">
      <div class="euiFlexItem euiFlexItem--flexGrowZero">
        <div class="euiFormRow">
          <!-- Form row stuff -->
        </div>
      </div>

      <div class="euiFlexItem euiFlexItem--flexGrowZero">
        <!-- Action -->
        <button>
          Next step
        </button>
      </div>
    </div>

It will look roughly like this:

image

ng-if="controller.isSystemIndicesCheckBoxVisible()"
class="kuiCheckBoxLabel"
for="indexPatternCreationIncludeSystemIndices"
<h1 class="euiTitle">
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we change lines 8-35 to:

      <!-- Intro -->
      <h1 class="euiTitle euiTitle--medium">
        Create index pattern
      </h1>

      <div class="euiSpacer euiSpacer--small"></div>

      <div class="euiFlexGroup euiFlexGroup--gutterLarge euiFlexGroup--justifyContentSpaceBetween euiFlexGroup--alignItemsEnd euiFlexGroup--responsive">
        <div class="euiFlexItem euiFlexItem--flexGrowZero">
          <div class="euiText">
            <p class="euiTextColor euiTextColor--subdued">
              Kibana uses index patterns to retrieve data from Elasticsearch indices for things like visualizations.
            </p>
          </div>
        </div>

        <div class="euiFlexItem euiFlexItem--flexGrowZero">
          <label
            ng-if="controller.isSystemIndicesCheckBoxVisible()"
            class="kuiCheckBoxLabel "
            for="indexPatternCreationIncludeSystemIndices"
          >
            <input
              type="checkbox"
              id="indexPatternCreationIncludeSystemIndices"
              class="kuiCheckBox"
              ng-model="controller.doesIncludeSystemIndices"
              ng-change="controller.onIncludeSystemIndicesChange()"
            >

            <span class="kuiCheckBoxLabel__text">
              Include system indices
            </span>
          </label>
        </div>
      </div>

This will render like this (a bit more space between the title and description):

image

@cjcenizal
Copy link
Contributor

We can also update Kibana to depend upon EUI v0.0.9. There are two breaking changes in that release which affect this PR:

  • Renamed euiFlexGroup--alignItemsEnd class to euiFlexGroup--alignItemsFlexEnd.
  • Remove support for primary color from <EuiTextColor> because it looked too much like a link.

Copy link
Contributor

@bmcconaghy bmcconaghy left a comment

Choose a reason for hiding this comment

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

LGTM after making the changes CJ recommends.

Copy link
Contributor

@cjcenizal cjcenizal left a comment

Choose a reason for hiding this comment

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

We're almost there! I found a few more things and then I believe we will be good to go.

indices="stepIndexPattern.allIndices"
pattern="stepIndexPattern.indexPatternName"
>
<span class="euiTextColor euiTextColor--primary">
Copy link
Contributor

Choose a reason for hiding this comment

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

We need to remove references to euiTextColor--primary because it's going to be removed from EUI.

indices="stepIndexPattern.partialMatchingIndices"
pattern="stepIndexPattern.indexPatternName"
>
<span class="euiTextColor euiTextColor--primary">
Copy link
Contributor

Choose a reason for hiding this comment

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

Here too.

<button
data-test-subj="createIndexPatternCreateButton"
ng-disabled="!stepTimeField.canCreateIndexPattern()"
class="euiButton euiButton--primary"
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we add euiButton--fill here to make it clearer that this is the button that completes this process?

Index pattern
</label>
<div>
<h2 class="euiTitle">
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we add euiTitle--small here?

</h2>
</div>
</div>
<h2 class="euiTitle">
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we add euiTitle--small here?

Refresh
</a>

<p
Copy link
Contributor

Choose a reason for hiding this comment

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

We can replace this element and its children with this EUI spinner instead:

          <div
            ng-if="stepTimeField.isFetchingTimeFieldOptions"
            class="euiLoadingSpinner euiLoadingSpinner--small"
            aria-label="Please wait while we fetch your time field options"
            aria-describedby="timeFilterFieldSelectLabel"
          ></div>

<div class="euiFlexItem euiFlexItem--flexGrowZero">
<a
ng-if="!stepTimeField.isFetchingTimeFieldOptions"
class="euiLink euiText euiText--extraSmall"
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we add this class here?

class="euiLink euiText euiText--extraSmall timeFieldRefreshButton"

And then in step_time_field.less we can add the class:

/**
 * 1. Bring the line-height down or else this link expands its container when it becomes visible.
 */
.timeFieldRefreshButton {
  line-height: 1 !important; /* 1 */
}

There was a minor layout shift which occurs when you click the refresh button and this change will fix it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

<div class="euiFlexGroup euiFlexGroup--gutterSmall euiFlexGroup--justifyContentFlexEnd">
<div class="euiFlexItem euiFlexItem--flexGrowZero">
<button
class="euiButtonEmpty euiButtonEmpty--primary kuiButton--iconText"
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we remove kuiButton--iconText from this element?

<!-- User has no data -->
<div ng-if="!controller.hasIndices()">
<div class="euiPageContentBody">
<div class="euiFlexGroup euiFlexGroup--gutterLarge euiFlexGroup--justifyContentCenter euiFlexGroup--alignItemsCenter euiFlexGroup--responsive" style="height:100%">
Copy link
Contributor

@cjcenizal cjcenizal Dec 22, 2017

Choose a reason for hiding this comment

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

Can we replace this entire div with this:

            <div class="euiFlexGroup euiFlexGroup--gutterLarge euiFlexGroup--alignItemsCenter euiFlexGroup--responsive">
              <!-- Fetching state -->
              <div
                ng-if="controller.isFetchingExistingIndices"
                class="euiFlexItem euiFlexItem--flexGrowZero createIndexPatternPrompt"
              >
                <h2 class="euiTitle euiTextColor euiTextColor--subdued">
                  Checking for Elasticsearch data
                </h2>
                <div class="euiSpacer euiSpacer--s"></div>
                <div class="euiFlexGroup euiFlexGroup--gutterSmall euiFlexGroup--alignItemsCenter">
                  <div class="euiFlexItem euiFlexItem--flexGrowZero">
                    <div
                      class="euiLoadingSpinner euiLoadingSpinner--medium"
                      aria-hidden="true"
                    ></div>
                  </div>
                  <div class="euiFlexItem euiFlexItem--flexGrowZero">
                    <p class="euiTextColor euiTextColor--subdued">
                      Reticulating splines...
                    </p>
                  </div>
                </div>
              </div>

              <!-- Empty state -->
              <div
                class="euiFlexItem euiFlexItem--flexGrowZero createIndexPatternPrompt"
                ng-if="!controller.isFetchingExistingIndices"
              >
                <div>
                  <h2 class="euiTitle euiTextColor euiTextColor--subdued">
                    Couldn't find any Elasticsearch data
                  </h2>
                  <div class="euiSpacer euiSpacer--s"></div>
                  <p class="euiTextColor euiTextColor--subdued">
                    <span>
                      You'll need to index some data into Elasticsearch before you can create an index pattern.
                    </span>
                    <a
                      class="kuiLink"
                      aria-label="Learn how to index data into Elasticsearch"
                      documentation-href="indexPatterns.loadingData"
                      target="_blank"
                      rel="noopener noreferrer"
                    >
                      Learn how.
                    </a>
                  </p>
                  <div class="euiSpacer euiSpacer--s"></div>
                  <button
                    class="euiButtonEmpty euiButtonEmpty--primary"
                    ng-click="controller.fetchExistingIndices()"
                  >
                    <span class="euiButton__content">
                      <span>
                        Check for new data
                      </span>
                    </span>
                  </button>
                </div>
              </div>
            </div>

This will:

  • Use h2 elements instead of h1.
  • Uses a primary empty button to check for new data.
  • Aligns text to the left so that the layout doesn't shift around so much when switching between the fetching and empty states.
  • Shows a EUI spinner which the indices are being fetched.
  • Adds another div inside of the loading state div to prevent the content from being laid out with flexbox rules. And then I also changed the style of the button to be empty and primary.

We also need to create create_index_pattern_wizard.less and add this class to make sure the empty and loading states don't get too wide:

.createIndexPatternPrompt {
  width: 100%;
  max-width: 600px;
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice, good idea!

Kibana uses index patterns to retrieve data from Elasticsearch indices for things like visualizations.
</p>
<div class="euiFlexItem euiFlexItem--flexGrowZero">
<label
Copy link
Contributor

Choose a reason for hiding this comment

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

We can convert this to an EUI checkbox by replacing this label and its contents with this:

          <div
            class="euiCheckbox"
            ng-if="controller.isSystemIndicesCheckBoxVisible()"
          >
            <input
              type="checkbox"
              class="euiCheckbox__input"
              id="indexPatternCreationIncludeSystemIndices"
              ng-model="controller.doesIncludeSystemIndices"
              ng-change="controller.onIncludeSystemIndicesChange()"
            >
            <div class="euiCheckbox__square">
              <div class="euiCheckbox__check"></div>
            </div>
            <label
              class="euiCheckbox__label"
              for="indexPatternCreationIncludeSystemIndices"
            >
              Include system indices
            </label>
          </div>

@chrisronline chrisronline force-pushed the eui/index-pattern-creation branch from 4f32a59 to 47b245c Compare December 22, 2017 18:53
@chrisronline
Copy link
Contributor Author

@cjcenizal I'm not in love with how pagination is working. I tried to adapt to the original design but wanted to use the newer UX as well. Thoughts on something better?

@cjcenizal
Copy link
Contributor

@chrisronline I think it's perhaps less than ideal but it's not bad, and it's also the best we can do with the current components. This might be something for @snide to consider. Perhaps we need to support a simplified pagination component or pattern for when the user just needs to be able to go back and forward. Two arrows like the KUI pagination maybe?

image

Copy link
Contributor

@cjcenizal cjcenizal left a comment

Choose a reason for hiding this comment

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

3 last minute requests and then let's MERGE THIS BABY!! 🔥

<!-- Action -->
<button
data-test-subj="createIndexPatternGoToStep2Button"
class="euiButton euiButton--primary euiButton--iconText"
Copy link
Contributor

Choose a reason for hiding this comment

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

euiButton--iconText doesn't exist in EUI so we can remove it.

ng-if="!controller.isFetchingExistingIndices"
>
<div>
<h2 class="euiTitle euiTextColor euiTextColor--subdued">
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we add euiTitle--small here?

ng-if="controller.isFetchingExistingIndices"
class="euiFlexItem euiFlexItem--flexGrowZero createIndexPatternPrompt"
>
<h2 class="euiTitle euiTextColor euiTextColor--subdued">
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we add euiTitle--small here?

@chrisronline chrisronline merged commit 6fb7d52 into elastic:master Jan 2, 2018
@chrisronline chrisronline deleted the eui/index-pattern-creation branch January 2, 2018 15:40
chrisronline added a commit to chrisronline/kibana that referenced this pull request Jan 2, 2018
…lastic#15722)

* First pass at applying EUI styles to the index pattern creation page

* PR feedback

* PR feedback

* PR feedback
chrisronline added a commit that referenced this pull request Jan 2, 2018
…15722) (#15810)

* First pass at applying EUI styles to the index pattern creation page

* PR feedback

* PR feedback

* PR feedback
@chrisronline
Copy link
Contributor Author

Backport:

6.x: 54676a8

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.

3 participants