Added documentation for htmlIdGenerator#3076
Conversation
|
Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually? |
chandlerprall
left a comment
There was a problem hiding this comment.
Some comments that apply to the suffix/prefix examples,
- Use
EuiFormRowto provide a label for the input boxes; placeholder text should be used to provide example data, not to label the field - Use
EuiFieldTextinstead ofEuiFieldSearch - I think it'd be nice if the examples pre-populated the input boxes to better demonstrate their use, instead of leaving them unused by default.
|
I have made the requested changes. Also, I have removed the reGenerate button so that user can see live changes with clicking on the button unnecessarily |
chandlerprall
left a comment
There was a problem hiding this comment.
Let's trim the examples down to one generated ID per section; having a mix of 1, 2, and 3 examples is a little confusing
|
All changes have been made |
|
Jenkins test this |
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_3076/ |
src-docs/src/views/html_id_generator/html_id_generator_example.js
Outdated
Show resolved
Hide resolved
src-docs/src/views/html_id_generator/html_id_generator_example.js
Outdated
Show resolved
Hide resolved
src-docs/src/views/html_id_generator/html_id_generator_example.js
Outdated
Show resolved
Hide resolved
src-docs/src/views/html_id_generator/html_id_generator_example.js
Outdated
Show resolved
Hide resolved
src-docs/src/views/html_id_generator/html_id_generator_example.js
Outdated
Show resolved
Hide resolved
Co-Authored-By: Caroline Horn <549577+cchaos@users.noreply.github.com>
Co-Authored-By: Caroline Horn <549577+cchaos@users.noreply.github.com>
Co-Authored-By: Caroline Horn <549577+cchaos@users.noreply.github.com>
Co-Authored-By: Caroline Horn <549577+cchaos@users.noreply.github.com>
Co-Authored-By: Caroline Horn <549577+cchaos@users.noreply.github.com>
Co-Authored-By: Caroline Horn <549577+cchaos@users.noreply.github.com>
Co-Authored-By: Caroline Horn <549577+cchaos@users.noreply.github.com>
Co-Authored-By: Caroline Horn <549577+cchaos@users.noreply.github.com>
Co-Authored-By: Caroline Horn <549577+cchaos@users.noreply.github.com>
Co-Authored-By: Caroline Horn <549577+cchaos@users.noreply.github.com>
|
@cchaos I have added description, |
|
I have updated it accordingly |
cchaos
left a comment
There was a problem hiding this comment.
@chandlerprall I don't think the prefix with suffix example is working properly. The function never produces a generated string. It just concatenates the prefix with an underscore then suffix.
So doing htmlIdGenerator('Some')('Id') produces Some_Id.
src-docs/src/views/html_id_generator/html_id_generator_example.js
Outdated
Show resolved
Hide resolved
src-docs/src/views/html_id_generator/html_id_generator_example.js
Outdated
Show resolved
Hide resolved
src-docs/src/views/html_id_generator/html_id_generator_example.js
Outdated
Show resolved
Hide resolved
Co-Authored-By: Caroline Horn <549577+cchaos@users.noreply.github.com>
I think this should not be the desired behaviour |
|
Heh, you're right. Didn't realize the original function worked that way that - we should continue supporting the existing test cases, including the original logic of |
This was the main cause to change the function, currently if no suffix is provided it generates if there is a suffix it renders It also includes the logic Earlier also if no prefix and no suffix was provided it generated |
|
Ach, thank you. This is an annoying set of rules! The following passes tests and the values shown in the docs appear correct export function htmlIdGenerator(idPrefix: string = '') {
const staticUuid = uuid.v1();
return (idSuffix: string = '') => {
const prefix = `${idPrefix}${idPrefix !== '' ? '_' : 'i'}`;
const suffix = idSuffix ? `_${idSuffix}` : '';
return `${prefix}${suffix ? staticUuid : uuid.v1()}${suffix}`;
};
} |
|
The generator function has been updated. |
|
jenkins test this |
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_3076/ |
chandlerprall
left a comment
There was a problem hiding this comment.
Functionality & docs interactions LGTM
src-docs/src/views/html_id_generator/html_id_generator_example.js
Outdated
Show resolved
Hide resolved
|
One last, jenkins test this |
|
Preview documentation changes for this PR: https://eui.elastic.co/pr_3076/ |
…fix` and `suffix` (elastic#3076)
Summary
Fixes : #639
added documentation for htmlIdGenerator with all cases
Checklist
- [ ] Check against all themes for compatibility in both light and dark modes- [ ] Checked in mobile- [ ] Checked in IE11 and Firefox- [ ] Props have proper autodocs- [ ] Added or updated jest tests- [ ] Checked for breaking changes and labeled appropriately- [ ] Checked for accessibility including keyboard-only and screenreader modes- [ ] A changelog entry exists and is marked appropriately