Skip to content

[canvas] Restore Workpad Title/button to Home; fix mounting behavior#103601

Merged
clintandrewhall merged 4 commits intoelastic:masterfrom
clintandrewhall:canvas/home-fixes
Jun 30, 2021
Merged

[canvas] Restore Workpad Title/button to Home; fix mounting behavior#103601
clintandrewhall merged 4 commits intoelastic:masterfrom
clintandrewhall:canvas/home-fixes

Conversation

@clintandrewhall
Copy link
Contributor

@clintandrewhall clintandrewhall commented Jun 29, 2021

Summary

#102446 removed the Canvas title in the lower-left, as it opened a Workpad dialog that we removed, and the breadcrumb navigation provided the same functionality. Thinking back on it, it made sense to restore that button, but just have it also link home.

Screen Shot 2021-06-28 at 9 17 42 PM

Screen Shot 2021-06-28 at 9 20 18 PM

This PR also fixes an issue where the initial load of Workpads fires too many times, (as the onMount hook doesn't fire only once).

@clintandrewhall clintandrewhall requested review from a team as code owners June 29, 2021 01:20
@clintandrewhall clintandrewhall added Feature:Canvas impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. loe:small Small Level of Effort review Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas t// v7.14.0 v8.0.0 labels Jun 29, 2021
@elasticmachine
Copy link
Contributor

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

@clintandrewhall clintandrewhall added auto-backport Deprecated - use backport:version if exact versions are needed release_note:skip Skip the PR/issue when compiling release notes labels Jun 29, 2021
Copy link
Contributor

@mdefazio mdefazio left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@cqliu1 cqliu1 left a comment

Choose a reason for hiding this comment

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

It feels a little odd to redirect the user back to the Canvas home page when they click on the workpad title on the bottom left. For a user new to Canvas, I think I'd be a little confused why clicking on the title in the bottom left backed me out of the workpad, since that's not what the workpad title breadcrumb does when you click on it (which just reloads the same workpad).

I'd be in favor of leaving it removed or making it static text, and have the users only navigate via the breadcrumbs. I don't believe there's a button to redirect you back to the dashboard listing when you're in a dashboard besides the breadcrumbs, and I think it'd be fine to expect the same in Canvas.

On that note looking in Dashboard, it looks like the dashboard title breadcrumb isn't clickable, so we should probably follow the same pattern and make the workpad title breadcrumb just static text.

The rest of the code LGTM 👍

@clintandrewhall
Copy link
Contributor Author

clintandrewhall commented Jun 29, 2021

@cqliu1 I agree, and made the changes you suggested. Thanks!

Screen Shot 2021-06-29 at 6 01 31 PM

@clintandrewhall clintandrewhall enabled auto-merge (squash) June 29, 2021 22:01
@clintandrewhall
Copy link
Contributor Author

jenkins test this

@clintandrewhall
Copy link
Contributor Author

@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
canvas 1044 1042 -2

Async chunks

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

id before after diff
canvas 1.2MB 1.2MB +349.0B

History

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

@kibanamachine
Copy link
Contributor

💚 Backport successful

Status Branch Result
7.x

This backport PR will be merged automatically after passing CI.

kibanamachine added a commit that referenced this pull request Jun 30, 2021
…103601) (#103824)

Co-authored-by: Clint Andrew Hall <clint.hall@elastic.co>
@clintandrewhall clintandrewhall deleted the canvas/home-fixes branch July 9, 2021 16:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto-backport Deprecated - use backport:version if exact versions are needed Feature:Canvas impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. loe:small Small Level of Effort release_note:skip Skip the PR/issue when compiling release notes review Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas t// v7.14.0 v8.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants