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

Support for searching for previous event in the Events Table #431

Merged
merged 1 commit into from
Aug 11, 2021

Conversation

bhufmann
Copy link
Collaborator

  • Add a 'previous' button in the column search cells right beside the
    'next' button
  • Use keyboard short-cut SHIFT+ENTER to search previous event

fixes #412

Signed-off-by: Bernd Hufmann Bernd.Hufmann@ericsson.com

@bhufmann bhufmann requested a review from PatrickTasse July 27, 2021 19:47
PatrickTasse
PatrickTasse previously approved these changes Jul 28, 2021
@bhufmann bhufmann requested a review from a user July 30, 2021 20:03
@bhufmann
Copy link
Collaborator Author

@muddana-satish could you please review this change? The latest nightly build has the corresponding back-end code implementation.

https://download.eclipse.org/tracecompass.incubator/trace-server/rcp/

ghost
ghost previously approved these changes Aug 2, 2021
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Looks good to me.

I don't know if we want to fix it in this but we no longer need the 'isFiltered' flag. Also we don't need additional information from the api like 'tags'.

@bhufmann bhufmann dismissed stale reviews from ghost and PatrickTasse via e7989cb August 2, 2021 20:30
@bhufmann
Copy link
Collaborator Author

bhufmann commented Aug 3, 2021

I updated the patch according to the review comments.

I also noticed a general bug with the searching. The AgGrid keeps a cache of size maxBlocksInCachecacheBlockSize (in our case (5200)=1000), where each block of cacheBlockSize events are fetched from the back-end. These 5 blocks, however, are not necessarily consecutive blocks, it depends on user's navigation. In the search feature, where we do this.gridApi.forEachNode(...) to move to the next match, the whole cache is looped through. It could happen that the first matched event in the cache is not inside the current block and the table moves somewhere else in the table where the next matched event in the cache is found. I corrected that in this PR as well.

Copy link
Contributor

@PatrickTasse PatrickTasse left a comment

Choose a reason for hiding this comment

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

Some more suggestions perhaps for future enhancement:

  • It should be possible to use Enter/Shift+Enter when the table itself has focus
  • There should be a visual feedback while we are searching the next/previous index, not only when we are waiting to fill the table at the found index
  • It should not lose current table selection if there is no found index

@bhufmann bhufmann self-assigned this Aug 4, 2021
- Add a 'previous' button in the column search cells right beside the
'next' button
- Use keyboard short-cut SHIFT+ENTER to search previous event

fixes eclipse-cdt-cloud#412

Signed-off-by: Bernd Hufmann <Bernd.Hufmann@ericsson.com>
Signed-off-by: Patrick Tasse <patrick.tasse@ericsson.com>
@Rodrigoplp-work Rodrigoplp-work merged commit 8f8159d into eclipse-cdt-cloud:master Aug 11, 2021
@bhufmann bhufmann deleted the table_search branch November 18, 2021 14:47
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.

Support searching for previous events in the events table
3 participants