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.

Sign up for GitHub

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

Adds getAriaName function and applies it to advanced settings #13448

Merged
merged 1 commit into from
Aug 16, 2017
Merged

Adds getAriaName function and applies it to advanced settings #13448

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/core_plugins/kibana/public/management/sections/settings/advanced_row.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@
ng-click="edit(conf)"
class="kuiMenuButton kuiMenuButton--basic kuiMenuButton--iconText"
ng-disabled="conf.tooComplex"
aria-label="Edit {{conf.ariaName}}"
data-test-subj="advancedSetting-{{conf.name}}-editButton"
>
<span
Expand All @@ -146,6 +147,7 @@
ng-click="save(conf)"
class="kuiMenuButton kuiMenuButton--primary kuiMenuButton--iconText"
ng-disabled="conf.loading || conf.tooComplex || forms.configEdit.$invalid"
aria-label="Save edit"
data-test-subj="advancedSetting-{{conf.name}}-saveButton"
>
<span
Expand All @@ -166,6 +168,7 @@
ng-if="!conf.editing"
ng-click="clear(conf)"
ng-hide="isDefaultValue(conf)"
aria-label="Clear {{conf.ariaName}}"
Copy link
Contributor

Choose a reason for hiding this comment

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

Would Reset {{conf.ariaName}} to default maybe be more descriptive to users?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Dunno. The button is labelled 'clear' so that's what I used for reference for the aria-label.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm fine with whatever you chose :)

class="kuiMenuButton kuiMenuButton--danger kuiMenuButton--iconText"
>
<span
Expand Down
26 changes: 26 additions & 0 deletions src/core_plugins/kibana/public/management/sections/settings/lib/__tests__/get_aria_name.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@

import { getAriaName } from '../get_aria_name';
import expect from 'expect.js';

describe('Settings', function () {
describe('Advanced', function () {
describe('getAriaName(name)', function () {
it('should be a function', function () {
expect(getAriaName).to.be.a(Function);
});

it('should return a space delimited lower-case string with no special characters', function () {
expect(getAriaName('xPack:defaultAdminEmail')).to.be('x pack default admin email');
expect(getAriaName('search:queryLanguage:switcher:enable')).to.be('search query language switcher enable');
expect(getAriaName('doc_table:highlight')).to.be('doc table highlight');
expect(getAriaName('foo')).to.be('foo');
});

it('should return an empty string if passed undefined or null', function () {
expect(getAriaName()).to.be('');
expect(getAriaName(undefined)).to.be('');
expect(getAriaName(null)).to.be('');
});
});
});
});
13 changes: 13 additions & 0 deletions src/core_plugins/kibana/public/management/sections/settings/lib/get_aria_name.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@

import { words } from 'lodash';

/**
* @name {string} the name of the configuration object
* @returns {string} a space demimited, lowercase string with
* special characters removed.
*
* Example: 'xPack:fooBar:foo_bar_baz' -> 'x pack foo bar foo bar baz'
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a list somewhere of all the possible advanced configs? I'm just hoping to verify this logic will hold true for all possible values

Copy link
Contributor

Choose a reason for hiding this comment

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

Agree, maybe we could make a text file or read it from there and utilize that complete list in the unit test. Otherwise this looks great!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think there is a master list. However, if there are specific cases you think we should test for, I can add additional tests. Seems overkill to process / maintain the entire list, when the logic really boils down to how it handles certain characters.

Copy link
Contributor

Choose a reason for hiding this comment

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

Fair!

Copy link
Contributor

Choose a reason for hiding this comment

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

Probably okay since we're not relying on a certain delimiter and I'm guessing/hoping the lodash words method is robust enough to handle any future settings

*/
export function getAriaName(name) {
return words(name).map(word => word.toLowerCase()).join(' ');
}
3 changes: 3 additions & 0 deletions src/core_plugins/kibana/public/management/sections/settings/lib/to_editable_config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { getValType } from './get_val_type';
import { getEditorType } from './get_editor_type';
import { getAriaName } from './get_aria_name';

/**
* @param {object} advanced setting definition object
Expand All @@ -13,6 +14,7 @@ export function toEditableConfig({ def, name, value, isCustom }) {
}
const conf = {
name,
ariaName: getAriaName(name),
value,
isCustom,
readonly: !!def.readonly,
Expand All @@ -33,3 +35,4 @@ export function toEditableConfig({ def, name, value, isCustom }) {

return conf;
}