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

Add search result statistics on the main search screen #16130

Merged

Conversation

maxiadlovskii
Copy link
Contributor

@maxiadlovskii maxiadlovskii commented Aug 8, 2023

Description

Pr adds
total results value to the sidebar execution info
Screenshot 2024-02-06 at 09 16 36
Search result statistics to search screen
Screenshot 2024-02-06 at 09 17 59
Execution info popover to each dashboard widget
Screenshot 2024-02-06 at 10 21 38

Motivation and Context

fix: Total result count missing from query page#7386

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Refactoring (non-breaking change)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.

/jenkins-pr-deps https://github.com/Graylog2/graylog-plugin-enterprise/pull/6564

@maxiadlovskii maxiadlovskii changed the title Add execution info components Add search result statistics on the main search screen Feb 6, 2024
@maxiadlovskii maxiadlovskii marked this pull request as ready for review February 6, 2024 09:26
@maxiadlovskii maxiadlovskii marked this pull request as draft February 6, 2024 12:12
@maxiadlovskii maxiadlovskii marked this pull request as ready for review February 6, 2024 15:55
const ExecutionInfo = () => {
const result = useAppSelector(selectCurrentQueryResults);
const total = result?.searchTypes && Object.values(result?.searchTypes)?.[0]?.total;

Copy link
Contributor

Choose a reason for hiding this comment

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

Currently we show "Query executed in 0ms" + "Total results: 0" when executing search for the first time.

Maybe let's unify it with the sidebar, where we show <i>No query executed yet.</i> in this case?


return (
<OverlayDropdown show={open}
toggleChild={<QueryHelpButton bsStyle="link"><Icon name="question-circle" /></QueryHelpButton>}
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be nice to use a button with a smaller height here, so the widget content receives more space.

Before
image

After:
image

Copy link
Contributor

Choose a reason for hiding this comment

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

I can imagine something like this
image

It could work well in combination with making the time range (5 minutes ago) + the icon the trigger to open the popover.

Copy link
Contributor

Choose a reason for hiding this comment

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

i also like to display the icon the gray we use for the widget actions, to put the focus on the widget content.


return (
<i>
Query executed in {' '}
Copy link
Contributor

Choose a reason for hiding this comment

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

The reads like we want to display two spaces at the end of the line.

}
const HelpPopover = ({ widgetExecutionData }: { widgetExecutionData: WidgetExecutionData}) => (
<PopoverContainer>
<p><strong>Execution Info</strong></p>
Copy link
Contributor

Choose a reason for hiding this comment

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

You can also just use the popover title prop, this way we unify the styling.

Copy link
Contributor

Choose a reason for hiding this comment

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

It would also be nice to remove the bottom margin:

image

Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like we just need to remove the table bottom margin
image

Copy link
Contributor

@linuspahl linuspahl left a comment

Choose a reason for hiding this comment

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

Regarding the dashboard widget popover toggle, it would be nice to move it 5 or even 10 pixel to the left so it wont be overlapped by the widget resize handle:

image

In addition we can use the same gray here, like the one we use for the widget action icons.

Maybe let's get rid of the table background color?
image

@linuspahl linuspahl merged commit ede6608 into master Feb 9, 2024
6 checks passed
@linuspahl linuspahl deleted the fix/Total-result-count-missing-from-query-page/issue-7386 branch February 9, 2024 14:31
@mpfz0r
Copy link
Contributor

mpfz0r commented Feb 15, 2024

💯 I love it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Total result count missing from query page
3 participants