-
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
De-angularize field-name directive #40744
De-angularize field-name directive #40744
Conversation
💔 Build Failed |
💚 Build Succeeded |
💚 Build Succeeded |
Pinging @elastic/kibana-app |
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 comment on sass change.
@@ -169,7 +169,7 @@ discover-app { | |||
|
|||
// SASSTODO: replace the margin-right value with a variables | |||
.dscField__icon { | |||
margin-right: 5px; | |||
margin-right: 10px; |
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.
Since you're changing this, can you go ahead and take on that todo? Likely if you're looking for a bit more space $euiSizeS
will work. It'll get you to 8px.
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.
Of course, we should further adapt this component, icons etc. but in a separate PR.
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.
i will adapt this change of course, more margin here is necessary, because react removed the whitespace angular added ... further work will include taking care of unused classes (for scripted fields e.g., guess there should be an icon for that etc.)
💚 Build Succeeded |
💚 Build Succeeded |
…cover-field-name-directive
💚 Build Succeeded |
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.
Ran on Mac/Chrome, everything seems to work.
…cover-field-name-directive
💚 Build Succeeded |
Summary
Convert the
field-name
directive from Angular to React. This is required to deangularizedoc_view
andfield_chooser
ofdiscover
.Part of #38646
Checklist
Use
strikethroughsto remove checklist items you don't feel are applicable to this PR.- [ ] Any text added follows EUI's writing guidelines, uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/master/packages/kbn-i18n/README.md)- [ ] Documentation was added for features that require explanation or tutorials- [ ] This was checked for keyboard-only and screenreader accessibilityFor maintainers
- [ ] This was checked for breaking API changes and was labeled appropriately- [ ] This includes a feature addition or change that requires a release note and was labeled appropriately