-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
WIP Accessibility palette swap #11610
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The buttons look so so much better. I had a few suggestions.
@@ -14,8 +14,8 @@ $localDropdownFormNoteTextColor: #737373; | |||
$localTabTextColor: $localNavButtonTextColor; | |||
$localTabTextColor-isHover: $localNavButtonTextColor-isHover; | |||
$localTabTextColor-isSelected: $localNavButtonTextColor-isHover; | |||
$localBreadcrumbLinkColor: #328caa; | |||
$localSearchButtonBackgroundColor: #9c9c9c; | |||
$localBreadcrumbLinkColor: $globalColorBlue; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
||
@include kuiButtonHoverAndActive { | ||
color: darken($globalLinkColor, 10%) !important; /* 1 */ | ||
border: solid 1px darken($globalColorBlue, 10%); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you an example of this button to the UI Framework? If you look in ui_framework/doc_site/src/views/button/
, you'll need to create a file similar to button_sizes.html
and add it to button_example.js
.
@@ -2,6 +2,7 @@ | |||
position: relative; | |||
padding: 10px $localNavSideSpacing 14px; | |||
background-color: $localDropdownBackgroundColor; | |||
border-bottom: solid 10px $globalColorLightGray; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@@ -15,18 +15,21 @@ | |||
border: 0; | |||
cursor: pointer; | |||
|
|||
&:hover { | |||
&:hover, &:focus { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small nit, but I've been stacking these to make them easier to scan:
&:hover,
&:focus {
Are you OK with keeping this convention?
@@ -18,7 +18,8 @@ | |||
border-radius: $globalBorderRadius; | |||
|
|||
@include kuiButtonDisabled { | |||
cursor: default; | |||
cursor: not-allowed; | |||
opacity: .5; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The disabled state doesn't quite look disabled in some contexts:
Some solutions off the top of my head:
- Change the context, e.g. make this sidebar have a white background.
- Change the button enabled state so it contrasts more, e.g. by adding a box-shadow or border. Then the disabled state will look "flatter".
- Change the button disabled state so it contrasts more, e.g. by also dropping the text opacity and/or by reducing the saturation of the button so it looks a little more grayed out.
<h5 ng-show="!field.details.error">Quick Count <kbn-info info="Top 5 values based on documents in the table" placement="right"></kbn-info> | ||
<span ng-if="!field.details.error" class="small discover-field-details-count"> | ||
( | ||
<p class="kuiSubduedText" ng-show="!field.details.error"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's add kuiSubText
class here to remove the bottom margin and preserve the font-size.
@@ -44,7 +43,7 @@ <h5 ng-show="!field.details.error">Quick Count <kbn-info info="Top 5 values base | |||
<a | |||
ng-href="{{field.details.visualizeUrl}}" | |||
ng-show="field.visualizable" | |||
class="kuiButton kuiButton--primary kuiButton--small kuiButton--fullWidth kuiVerticalRhythmSmall" | |||
class="kuiButton kuiButton--secondary kuiButton--small kuiButton--fullWidth kuiVerticalRhythmSmall" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A couple things bother me about this button... the white background feels out of place because it doesn't match the background of the panel it's in, and the hover state doesn't feel very noticeable to me:
I think we should make the background semi-transparent, so it will adjust to the background of the container. And then what if we did a background change to make the hover state more noticeable? This is similar to how this button on elastic.co behaves:
I just threw this together to communicate the idea, I'm sure it could be improved:
Works on white too:
Here's the SCSS I used:
.kuiButton--secondary {
color: $globalLinkColor !important;
border: solid 1px $globalColorBlue;
background-color: rgba($globalColorWhite, 0.4);
@include kuiButtonHoverAndActive {
color: darken($globalLinkColor, 10%) !important;
border: solid 1px darken($globalColorBlue, 10%);
background-color: rgba(lighten($globalLinkColor, 50%), 0.1);
}
}
This looks great! @snide I noticed the loading indicator still has some pink in it |
Honestly, I prefer the teal logo treatment to this new one. I just kind of think the new one seems out of place with the current color scheme - perhaps the lighter background makes it look like it belongs more to the main content area than the sidebar, so it kind of looks like the sidebar just begins out of nowhere. I know we're trying not to overthink this too much, but at the same time this is going to be the Kibana that people use every single day for a year or more, so it should be something we're happy with. The Kibana 5 design is far from perfect, but it was such a better design than Kibana 4 that any problems it has are totally worth living with. Anyone have thoughts here? I don't have a useful alternative to propose at this point, but I'll think about it. |
A few of us discussed this in a group and decided the best option for now would be to have the logo sit on the same blue background as the rest of the sidebar, and the logo itself would just be the flat white logo rather than the multi-color logo. |
@snide What's the status of this? If it's ready for review, can you rebase on or merge master to resolve conflicts, take the WIP label out of the title, slap a review label on this, and update the PR description with the latest information about what's in this PR/updated images/etc? |
@epixa Not ready for review. It needs a day or two of solid love, mostly going through some more coloring, and then some refactoring on buttons for KUI. Unfortunately didn't have much time for it with Xschool this week. It's next on my plate. I'll see where I can get it today. |
) * Implement a way to hide write controls from dashboard * Add roles definition to the saved dashboard object * Remove roles from dashboard * Use an angular provider to control the hideWriteControls flag * Add an api to chrome to show only a single app by id * Refactor injectVars logic * Fix tests * Refactor kibana root controller * add additional test functionality * Missed a couple spots for hiding edit controls * add retry and help text for dash only mode failures See if it helps. Tests pass locally. * Move element into screen when clicking * use shorthand for object creation * Hide checkboxes, show 'no dashboard' message with the button
- Removing "I'm a pro" link from top right. - Adding "Got it, take me to Kibana" button at bottom center. - De-emphasizing demo site.
…lastic#11969) * [indexPatterns/field] replace mapping awareness with readFromDocValues * move link to more details to proper location * Address kjbekkelund's feedback * let the prs speak for themselves
All builds are now 64 bit, which is what we want to support from 6.0 onward.
* [indexPatterns/create] timeFields -> timeFieldOptions In elastic#11409 we added two new possible options to the "Time Filter field name" drop down. These options display a message that "there are no time fields" or "I don't want to use the time filter". These new options were added to the `controller.timeFields` array, which is where the other options were, but they don't actually represent time fields. This change updates `controller.timeFields` to instead be a list of options, specifically created to populate the select box in the view. They are now stored at `controller.timeFieldOptions` and each option has a `display` property (which will be rendered in the view) and optionally a `fieldName` property. The selected option, `controller.formValues.timeFieldOption` is checked for a `fieldName` property when the Index Pattern is created. * [indexPatterns/create] populate `this.timeFieldOptions` * [indexPatterns/create] unify fetching and errors for timeFieldOptions * [indexPatterns/create] refreshFieldList() -> refreshTimeFieldOptions() * [indexPatterns/create] unify "No time fields available" messages * [indexPatterns/create] disable the timeField select when loading * [indexPatterns/create] only require a time field is there are options Doing this prevents the time field from rendering as invalid when there is no matching index pattern (which has it's own error display) and when the fields are loading * [indexPatterns/create] isolate side-effects * [indexPatterns/create] let refreshTimeFieldOptions() control model * [indexPatterns/create] attempt to make default field selection clearer * [indexPatterns/create] explain why we set timeFieldOption * [indexPatterns/create] make timeFieldName `undefined` when not set * [indexPatterns/create] Don't pass two args to .then() unless needed * 💅
With full screen panel mode, the width and height can be updated to 0 (because the panel is hidden). If _size isn’t updated, the second time through it’ll skip the resize because of the if (newWidth === this._size[0] && newHeight === this._size[1]) Another option to fix this is to get rid of that check, and continue to do the resize when the _size values are equal. I felt this one was slightly better, but could go either way.
Using the --dev flag is the only support way to change the internal Kibana environment to something else. Allowing NODE_ENV to be set at all can have unexpected results with our dependencies, so we now always set it to production.
* Fix a type error from the default to named conversion add tests that would have caught it. * address code review comments
* change type property to buttonType * Update the tests to assert that the type HTML attribute is supported * remove type=submit from KuiSubmitButton since component automatically sets type
…12040) * When on an embedded page, bypass Getting Started gate check * Add unit test for embedded page scenario
…board Panel (elastic#12030) * Move drag handle attribute onto the button element itself within Dashboard Panel. Fixes bug in which Firefox users could no longer drag the panel. * Use anchor elements for all Dashboard Panel buttons for consistent hover and focus states.
* [grunt/jest] fail task when jest fails * 😶
* Fix use of KuiButtons in Clone Modal. * [scripts] stub scripts/languages api until elastic#11959 is fixed
This reverts commit 71a9b8b. Until we get all the relevant infra configured with yarn as well.
Prior to version 4.2.0, kibana.yml configurations used underscore as a separator. In 4.2.0, this changed to dot instead, though the old configuration names continued to work. In 5.0 we started logging deprecation notices whenever the underscore-separated configurations were encountered. Now we remove support for them entirely.
…c#12011) * [indexPatterns/create] disable time-based options when needed * !! -> Boolean() * [indexPatterns/create] refactor sample deduping for clarity * [indexPattern/create] wrap html attributes
Messed up my rebase, replaced with #12085 |
WIP, don't merge.
Addresses #11521