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

[Cases]: Redesign all cases list select modal #149851

Merged
merged 23 commits into from
Feb 6, 2023

Conversation

js-jankisalvi
Copy link
Contributor

@js-jankisalvi js-jankisalvi commented Jan 30, 2023

Summary

This PR Fixes: #139194
I Removes below columns and filters from Cases select modal.

Columns

  • Tags
  • Reporters/Assignees
  • Solution
  • Comments
  • Alerts
  • Updated On
  • Status
  • External Incident

Filters

  • Reporters/Assignees
  • Create case button (Create case button will be visible when there are no cases available)

Before
image

After: Updated
image

image

Flaky test runner: https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/1867

Checklist

Delete any items that are not applicable to this PR.

For maintainers

@js-jankisalvi js-jankisalvi added release_note:skip Skip the PR/issue when compiling release notes backport:skip This commit does not require backporting Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) Feature:Cases Cases feature labels Jan 30, 2023
@js-jankisalvi js-jankisalvi self-assigned this Jan 30, 2023
@js-jankisalvi js-jankisalvi added v8.7.0 release_note:enhancement and removed release_note:skip Skip the PR/issue when compiling release notes labels Feb 1, 2023
@js-jankisalvi js-jankisalvi requested a review from a team February 1, 2023 13:16
@js-jankisalvi js-jankisalvi marked this pull request as ready for review February 1, 2023 13:19
@js-jankisalvi js-jankisalvi requested a review from a team as a code owner February 1, 2023 13:19
@elasticmachine
Copy link
Contributor

Pinging @elastic/response-ops (Team:ResponseOps)

@elasticmachine
Copy link
Contributor

Pinging @elastic/response-ops-cases (Feature:Cases)

Copy link
Contributor

@jonathan-buttner jonathan-buttner left a comment

Choose a reason for hiding this comment

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

Looking good. Left a few questions and suggestions.

I tried adding a case with a long name and it seems like we have a good amount of whitespace, should we make more room for the title?

image

Copy link
Contributor

@peteharverson peteharverson left a comment

Choose a reason for hiding this comment

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

I like the new cleaner design with fewer columns!

Agree with @jonathan-buttner about giving the Name column more room, and also need to decide on what to do with the 'Create case' button.

@mdefazio
Copy link
Contributor

mdefazio commented Feb 2, 2023

Thank you for updating this! 🙌

Here's a quick mock with the addition of 'create case' option. We typically show the create button at the beginning of the search bar so not to confuse it with a 'update' or 'submit' button that is usually at the end.

image

One small tweak I would also suggest from the original concept is to show the 'Select' buttons as the default (tinted background) styling, rather than the empty buttons. I think they should stand out more than they do currently.

I included the cancel footer (as an empty button), but i'm not sure you need it.

@js-jankisalvi
Copy link
Contributor Author

Thanks for quick update!

One small tweak I would also suggest from the original concept is to show the 'Select' buttons as the default (tinted background) styling, rather than the empty buttons. I think they should stand out more than they do currently.

Agree. Select buttons should stand out.

I included the cancel footer (as an empty button), but i'm not sure you need it.

I think we have close button at top right corner. So cancel is not needed.

Copy link
Contributor

@adcoelho adcoelho left a comment

Choose a reason for hiding this comment

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

Tested already the new design and works fine for me, good job! 👌

There are some jarring rerenders that the modal does whenever there are no cases displayed and we trigger filters or sorting.

If there are rows then the whole modal doesn't rerender.

Screen.Recording.2023-02-02.at.17.18.25.mov

But I think it has nothing to do with your changes, maybe I should create a new issue @peteharverson ?

Copy link
Contributor

@peteharverson peteharverson left a comment

Choose a reason for hiding this comment

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

Tested latest changes and the design changes LGTM

There are some jarring rerenders that the modal does whenever there are no cases displayed and we trigger filters or sorting.

Yes I see this too. Agree with @adcoelho that this deserves a separate issue

@js-jankisalvi
Copy link
Contributor Author

There are some jarring rerenders that the modal does whenever there are no cases displayed and we trigger filters or sorting.

Yes I see this too. Agree with @adcoelho that this deserves a separate issue

Created issue #150282

Copy link
Member

@cnasikas cnasikas left a comment

Choose a reason for hiding this comment

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

Great job! Tested and I working as expected. I left some comments.

@js-jankisalvi js-jankisalvi requested a review from a team as a code owner February 6, 2023 12:14
Copy link
Contributor

@semd semd left a comment

Choose a reason for hiding this comment

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

Security-Explore changes LGTM

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
cases 594 595 +1

Async chunks

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

id before after diff
cases 391.9KB 393.4KB +1.6KB

Page load bundle

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

id before after diff
cases 129.8KB 129.8KB +15.0B

History

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

cc @js-jankisalvi

@js-jankisalvi js-jankisalvi merged commit daf1304 into elastic:main Feb 6, 2023
@js-jankisalvi js-jankisalvi deleted the select-case-modal branch October 6, 2023 07:37
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 Feature:Cases Cases feature release_note:enhancement Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) v8.7.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Cases] Simplify information shown in the 'Select case' modal
10 participants