Skip to content

[Time To Visualize] Edit Panel Title On Click#81076

Merged
ThomThomson merged 8 commits intoelastic:masterfrom
ThomThomson:feature/editTitleOnClick
Oct 22, 2020
Merged

[Time To Visualize] Edit Panel Title On Click#81076
ThomThomson merged 8 commits intoelastic:masterfrom
ThomThomson:feature/editTitleOnClick

Conversation

@ThomThomson
Copy link
Contributor

Summary

Closes #81047

Adds an EuiLink to the title on panel headers which opens the title customization menu.

Known Issue
For some reason, when text is inside the EuiLink, the font-weight: 700 style on the title is lost

Oct-19-2020 17-50-20

For maintainers

@ThomThomson ThomThomson added release_note:enhancement Feature:Dashboard Dashboard related features Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas t// Project:TimeToVisualize labels Oct 19, 2020
@ThomThomson ThomThomson marked this pull request as ready for review October 21, 2020 15:04
@ThomThomson ThomThomson requested a review from a team as a code owner October 21, 2020 15:04
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-canvas (Team:Canvas)

Copy link
Contributor

@streamich streamich left a comment

Choose a reason for hiding this comment

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

Code LGTM.

What do you think about creating a new TITLE_CLICK_TRIGGER trigger, so that anyone would be able to attach actions to it?

@botelastic botelastic bot added the Feature:Embedding Embedding content via iFrame label Oct 21, 2020
@ThomThomson
Copy link
Contributor Author

The main idea with this one is to lower the number of clicks required - if we created a trigger, potentially there could be multiple actions registered correct? Then we'd have to render a popover to show them - which would kind of defeat the purpose.

That said, I do like how clean it would make the code - this PR has a wee bit of a code smell to be honest. Additionally, we could do something like if 1 action is registered, run it, if multiple actions are registered show a popover and try to ensure that people only use the TITLE_CLICK_TRIGGER if absolutely necessary.

@ryankeairns
Copy link
Contributor

ryankeairns commented Oct 21, 2020

@ThomThomson To resolve the bold font issue, just add a custom className to your EuiLink (e.g. embPanel__titleLink) and set this style in
src/plugins/embeddable/public/lib/panel/_embeddable_panel.scss:

.embPanel__titleLink {
   font-weight: $euiFontWeightBold;
}

Screen Shot 2020-10-21 at 2 02 39 PM

@ThomThomson ThomThomson requested a review from a team as a code owner October 22, 2020 15:28
@cchaos cchaos requested a review from ryankeairns October 22, 2020 17:44
@ryankeairns
Copy link
Contributor

@ThomThomson I opened a small design PR that slightly simplifies the class logic / uses the existing class nams vs introducing another one. If it looks good to you, then feel free to merge it in here and I'll stamp this PR.

@ThomThomson
Copy link
Contributor Author

@ryankeairns this is way more readable, thanks!

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

page load bundle size

id before after diff
embeddable 301.5KB 301.8KB +363.0B

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@ThomThomson ThomThomson merged commit 06ea880 into elastic:master Oct 22, 2020
ThomThomson added a commit to ThomThomson/kibana that referenced this pull request Oct 22, 2020
* Made embeddable panel title click launch the customize panel action

Co-authored-by: Ryan Keairns <contactryank@gmail.com>
@AlonaNadler
Copy link

wow that was fast!!

ThomThomson added a commit that referenced this pull request Oct 26, 2020
* Made embeddable panel title click launch the customize panel action

Co-authored-by: Ryan Keairns <contactryank@gmail.com>
@th0ger
Copy link

th0ger commented Mar 19, 2021

After upgrading to Kibana 7.11, I experienced that all panel titles that was disabled, had now been enabled.
I think that this feature may have been the cause.

@ThomThomson
Copy link
Contributor Author

@th0ger, this regression should be fixed in 7.12.1 by #95355

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Feature:Dashboard Dashboard related features Feature:Embedding Embedding content via iFrame Project:TimeToVisualize release_note:enhancement Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas t// v7.11.0 v8.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Abiltiy to change dashboard panel title name close to the title

7 participants