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

[Spaces management] make section panel title optional #190865

Conversation

tsullivan
Copy link
Member

@tsullivan tsullivan commented Aug 20, 2024

Summary

Epic link: https://github.com/elastic/kibana-team/issues/785

The Spaces Management SectionPanel component has been updated to make title an optional prop. This PR takes a small change out of a larger PR to be delivered next: #184697.

Currently, title is shown consistently for all SectionPanel elements in the Edit Space screen. In the new design, the panels will appear on a tabbed interface. Observe how the new design implemented in the larger/next PR will leave the SectionPanel title empty (this screenshot was taken when running Kibana in the branch for the next PR):
image

The current design will still be used for the Create Space interface. As a reminder, here is how the current design looks:
image

@tsullivan tsullivan added the release_note:skip Skip the PR/issue when compiling release notes label Aug 20, 2024
@tsullivan tsullivan marked this pull request as ready for review August 20, 2024 21:19
@tsullivan tsullivan requested a review from a team as a code owner August 20, 2024 21:19
@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Metrics [docs]

Async chunks

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

id before after diff
spaces 184.1KB 184.2KB +86.0B

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

@jeramysoucy jeramysoucy self-requested a review August 21, 2024 07:15
Copy link
Contributor

@jeramysoucy jeramysoucy left a comment

Choose a reason for hiding this comment

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

Implementation LGTM!

I am not entirely up to speed on this effort, so before I approve I wanted to go over a few things.

I was able to enable the solution view render with the following config options

xpack.cloud.onboarding.default_solution: "security"
xpack.cloud.id: "ftr_fake_cloud_id:aGVsbG8uY29tOjQ0MyRFUzEyM2FiYyRrYm4xMjNhYmM="
xpack.cloud.base_url: "https://cloud.elastic.co/"
xpack.cloud.deployment_url: "/deployments/deploymentId"
xpack.cloud_integrations.experiments.enabled: true
xpack.cloud_integrations.experiments.flag_overrides:
  "solutionNavEnabled": true

I am not sure all of those are actually needed, but this worked to enable the solution view render for spaces management.

The edit space screen remained unchanged in this PR (no avatar at top, no solution or current space badge, etc.) - I assume this is expected, and all of those changes are implemented in your other PR? Or am I missing part of the configuration settings?
(I did alter the source to trigger the behavior of no section titles and it worked).

Why do we want to hide the section titles only in Edit mode? Apologies, I am out of the loop on this effort.

@tsullivan
Copy link
Member Author

Thanks for the review, @jeramysoucy!

The edit space screen remained unchanged in this PR (no avatar at top, no solution or current space badge, etc.) - I assume this is expected, and all of those changes are implemented in your other PR? Or am I missing part of the configuration settings?

Yes, all of the visual changes will be implemented in my other PR: #184697. The flexibility created in this PR will be needed for the next one. The top screenshot in the description text was taken from running the next PR. I'm attempting to make the next PR slightly smaller by pulling things like this out into standalone PRs.

Why do we want to hide the section titles only in Edit mode? Apologies, I am out of the loop on this effort.

The next-ish PR will deliver the full UX improvements to Spaces, and will add the ability to assign Roles to a space when you edit the space. The space creation page will still be the same, but editing a space will feature a more complex UI. To manage that complexity, there will be a tabbed interface. When these controls are on a tab in the page, the design is a bit more compressed. I'm not sure if that completely explains why the design for the screen to edit general settings of the space will show the panels without titles, though. I expect the space creation page design may go through some changes in a future iteration of this project as well.

Sorry if my description text was confusing - I have updated it a bit.

@tsullivan tsullivan requested a review from jeramysoucy August 21, 2024 20:24
Copy link
Contributor

@jeramysoucy jeramysoucy left a comment

Choose a reason for hiding this comment

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

Thanks for the additional details, Tim! Having additional settings in the edit UI is reasonable to me, but not sure how I feel about the section title inconsistency from a UX perspective. If this is the approved design, please disregard my concern. And if not, can we tag the design team on the follow-up PR?

@tsullivan
Copy link
Member Author

Thanks @jeramysoucy. I have asked the project designer and PM to follow up with you on the question of whether the new design for the Edit Space screen will create inconsistency with the Create Space screen.

@tsullivan tsullivan merged commit 4b0f142 into elastic:main Aug 22, 2024
25 checks passed
@kibanamachine kibanamachine added v8.16.0 backport:skip This commit does not require backporting labels Aug 22, 2024
@tsullivan tsullivan deleted the spaces-management/make-section-panel-title-optional branch August 22, 2024 18:19
@tsullivan
Copy link
Member Author

@jeramysoucy here is a response I got from @ek-so, our project designer:

I imagined those 2 screens to be identical, that would be the right thing from UX point of view. Hopefully we can plan this as a follow-up task

The response included this screenshot:
image

@ek-so
Copy link
Contributor

ek-so commented Aug 23, 2024

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting release_note:skip Skip the PR/issue when compiling release notes v8.16.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants