-
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
Remove special handling of title
in base embeddable class
#63020
Closed
stacey-gammon
wants to merge
1
commit into
elastic:master
from
stacey-gammon:2020-04-06-no-get-title
Closed
Remove special handling of title
in base embeddable class
#63020
stacey-gammon
wants to merge
1
commit into
elastic:master
from
stacey-gammon:2020-04-06-no-get-title
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
bb6c1e4
to
345d1e0
Compare
345d1e0
to
5c0351e
Compare
💔 Build Failed
Failed CI Steps
Test FailuresKibana Pipeline / kibana-oss-agent / Example plugin functional tests.test/examples/embeddables/adding_children·ts.embeddable explorer creating and adding children Can create a new childStandard Out
Stack Trace
Kibana Pipeline / kibana-oss-agent / Chrome UI Functional Tests.test/functional/apps/dashboard/dashboard_options·js.dashboard app using current data dashboard data-shared attributes should be able to hide all panel titlesStandard Out
Stack Trace
Kibana Pipeline / kibana-oss-agent / Chrome UI Functional Tests.test/functional/apps/dashboard/dashboard_options·js.dashboard app using current data dashboard data-shared attributes should be able to hide all panel titlesStandard Out
Stack Trace
and 1 more failures, only showing the first 3. History
To update your PR or re-run it, just comment with: |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
A long term goal of mine is to replace the abstract base class Embeddable and make it an interface, and this is one small step towards that.
I found this
getTitle
abstraction to be confusing anyway, as what was actually being used was abstracted away. This PR removes that abstraction and just usesinput
andoutput
state directly.Instead of:
There is now:
EmbeddablePanel
uses these values to decide what to display:input.customPanelTitle
is defined, it is used. If it is an empty string, the title is hiddeninput.customPanelTitle
is not defined,EmbeddablePanel
checks for a value onoutput.panelTitle
.Initially I was thinking values on
output
should trump values on input, but I have since changed my mind. I think values oninput
are like overrides and if that is set, that is what should be shown.I think this logic is a bit simpler and no magic
getTitle
function. This also makes the values more explicitly named since the more generictitle
was conflicting with some of my examples that use atitle
value to render inside the Embeddable itself, not just on the EmbeddablePanel.