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

[Dashboard] Panel toolbar #83342

Merged
merged 6 commits into from
Dec 16, 2020
Merged

Conversation

cqliu1
Copy link
Contributor

@cqliu1 cqliu1 commented Nov 12, 2020

Summary

Blocked by #82909.
Closes #82906.
Related to #85666.

This adds a toolbar below the search bar and filter bar in Dashboard for creating new panels and adding existing embeddables.

The Create panel (previously Create new) button triggers the visualize modal for creating new visualizations.

The Add from library/Library (previously New) button triggers the embeddable flyout for adding Saved Object backed embeddables.

Screen Shot 2020-12-04 at 2 41 21 PM

Notable changes:

  • The dashboard title will no longer say Editing <dashboard-name> in edit mode. It will just say the title, i.e. New Dashboard or New Dashboard (unsaved)
  • Adds storybook support for dashboard

Note: This PR is based off #82909. View the last commit f0e731b (#83342) to see the diff for changes relevant to this PR.

Checklist

Delete any items that are not applicable to this PR.

For maintainers

@cqliu1 cqliu1 added the WIP Work in progress label Nov 12, 2020
@cqliu1 cqliu1 force-pushed the dashboard/add-panel-toolbar branch 3 times, most recently from ba7dfe1 to f22ab6d Compare November 18, 2020 19:28
@ryankeairns
Copy link
Contributor

ryankeairns commented Nov 19, 2020

Opened an initial design PR - cqliu1#15

As we continue down this path, we'll consider better alignment with the Lens toolbar. As it stands, the button sizes are smaller in Dashboard, we use a primary button, etc., but the rest didn't feel right for Dashboard at this juncture. Once we add more features - especially button groups - we can seek stronger alignment.

Screen Shot 2020-11-19 at 3 43 29 PM

@cqliu1 cqliu1 force-pushed the dashboard/add-panel-toolbar branch 2 times, most recently from 15ee66f to 5a5a604 Compare November 25, 2020 17:45
@cqliu1 cqliu1 added Feature:Dashboard Dashboard related features impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. loe:medium Medium Level of Effort release_note:enhancement Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas labels Nov 25, 2020
@cqliu1 cqliu1 force-pushed the dashboard/add-panel-toolbar branch from 01fec0f to 8e66f2a Compare November 30, 2020 21:25
@cqliu1 cqliu1 force-pushed the dashboard/add-panel-toolbar branch 4 times, most recently from fee1659 to f0e731b Compare December 10, 2020 02:01
@cqliu1 cqliu1 marked this pull request as ready for review December 10, 2020 02:03
@cqliu1 cqliu1 requested review from a team as code owners December 10, 2020 02:03
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-presentation (Team:Presentation)

@cqliu1 cqliu1 added review and removed WIP Work in progress labels Dec 10, 2020
Copy link
Contributor

@darnautov darnautov left a comment

Choose a reason for hiding this comment

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

ML changes LGTM

@botelastic botelastic bot added the Feature:Embedding Embedding content via iFrame label Dec 10, 2020
Copy link
Member

@jbudz jbudz left a comment

Choose a reason for hiding this comment

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

operations - storybook aliases LGTM

Copy link
Contributor

@ryankeairns ryankeairns 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!
Double checked the CSS, all still valid.
The button labels were reviewed by Gail ++

<FormattedMessage id="embeddableApi.addPanel.Title" defaultMessage="Add panels" />
<FormattedMessage
id="embeddableApi.addPanel.Title"
defaultMessage="Add from library"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@elastic/kibana-app-services This is the only change I made to the embeddable plugin which updates the title of the add panel flyout to align with the rename of the button in Dashboard to Add from library.

@kibanamachine
Copy link
Contributor

⏳ Build in-progress, with failures

Failed CI Steps

History

  • 💔 Build #93341 failed f0e731bfbdd097449c49430ac13882d8c41acf40
  • 💔 Build #93297 failed fee1659a92aa611f98f5f96af621cf50241d993d
  • 💔 Build #93250 failed 7745a663be560720de6aff686130b5c667865d9d
  • 💔 Build #92900 failed 1d9fcf713b8dccd99537accb8fe2a8a701e15688
  • 💔 Build #92159 failed 6058a4f4807d2bb22b11230447c596deb05d0638

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

Hide panel toolbar in view mode

Update empty panel copy

Added buttons to empty prompt

Update copy

Update toolbar and empty panel styles

Fixed some tests

Fixed conflict

Updated snapshots

Removed unused import

Added stories

Added storyshot test

Updated snapshots
@cqliu1 cqliu1 force-pushed the dashboard/add-panel-toolbar branch from 11c71cc to 001b335 Compare December 14, 2020 16:33
Copy link
Contributor

@ThomThomson ThomThomson left a comment

Choose a reason for hiding this comment

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

This is all looking great to me! Tested locally in chrome and everything seems to work as expected.

Not necessarily something to do in this PR, but I've opened up a conversation about the top nav here: #85666 (comment)

}
showLinkToVisualize={isEditMode}
onVisualizeClick={this.createNewEmbeddable}
onLinkClick={() => this.props.switchViewMode?.(ViewMode.EDIT)}
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm glad this PR removes the 'Create New' button from the empty screen - and the new design is looking great! If at all possible, maybe in a followup, it would also be great to remove the 'switchViewMode` behaviour. This method is injected in a non-standard way and is the only place where the dashboard state manager is coupled with the dashboard container.

onLibraryClick: () => void;
}

export const PanelToolbar: FC<Props> = ({ onAddPanelClick, onLibraryClick }) => (
Copy link
Contributor

Choose a reason for hiding this comment

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

This is looking good! Can't wait until we standardize this in the Presentation Team plugin

@cqliu1
Copy link
Contributor Author

cqliu1 commented Dec 16, 2020

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
dashboard 186 197 +11

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
dashboard 186.1KB 190.8KB +4.7KB

Distributable file count

id before after diff
default 47266 48027 +761
oss 27861 27862 +1

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
dashboard 336.1KB 333.5KB -2.5KB
embeddable 228.8KB 228.8KB +6.0B
total -2.5KB

History

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

@cqliu1 cqliu1 merged commit 92a805f into elastic:master Dec 16, 2020
@cqliu1 cqliu1 deleted the dashboard/add-panel-toolbar branch December 16, 2020 02:30
cqliu1 added a commit to cqliu1/kibana that referenced this pull request Dec 16, 2020
cqliu1 added a commit that referenced this pull request Dec 16, 2020
Co-authored-by: Kibana Machine <[email protected]>

Co-authored-by: Kibana Machine <[email protected]>
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 impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. loe:medium Medium Level of Effort release_note:enhancement review Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas v7.11.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

emphasize dashboard create new CTA
8 participants