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

[indexPatterns] update field in place #11966

Merged
merged 2 commits into from
May 24, 2017

Conversation

spalger
Copy link
Contributor

@spalger spalger commented May 23, 2017

When saving updates to a field in an index pattern the field is currently moved to the bottom of the field list, this change causes the field to be replaced in-place instead.

2017-05-22 17 01 50

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.

Looks great! Funny how such a small change makes such a big difference in the UX. Had one thought, but it's nothing earth-shaking.

fields.push(field);
const index = fields.findIndex(f => f.name === field.name);
if (index > -1) {
fields.splice(index, 1, field);
Copy link
Contributor

Choose a reason for hiding this comment

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

You could also assign to the index, might be simpler to read:

fields[index] = field;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call, didn't think about that. I'm generally hesitant to use assignment with arrays, but since the index is coming from .findIndex() it should be good, and I agree that it will be easier to read.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Welp, actually, since indexPatterns.fields is an indexed array assignment doesn't work 😬

@Bargs
Copy link
Contributor

Bargs commented May 23, 2017

@spalger I'm focusing on the cross cluster search PRs atm, you may wanna assign this to someone else if you'd like it reviewed in parallel otherwise it'll take me a couple few days to get to it.

@spalger
Copy link
Contributor Author

spalger commented May 24, 2017

@Bargs no worries, it's super small. I'm good with a single reviewer.

@spalger spalger force-pushed the fix/update-field-in-place branch from 8131fc3 to 52eb84c Compare May 24, 2017 02:13
@spalger spalger merged commit df2fb14 into elastic:master May 24, 2017
spalger added a commit that referenced this pull request May 24, 2017
@spalger
Copy link
Contributor Author

spalger commented May 24, 2017

5.5/5.x: fe5abb2

snide pushed a commit to snide/kibana that referenced this pull request May 30, 2017
@spalger spalger deleted the fix/update-field-in-place branch October 18, 2019 17:43
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