Skip to content

[OGUI-1453] Double click on value to move it to filter box#3463

Merged
isaachilly merged 27 commits into
devfrom
feature/ILG/OGUI-1453/Double-click-on-value-to-filter-with-it
May 22, 2026
Merged

[OGUI-1453] Double click on value to move it to filter box#3463
isaachilly merged 27 commits into
devfrom
feature/ILG/OGUI-1453/Double-click-on-value-to-filter-with-it

Conversation

@isaachilly
Copy link
Copy Markdown
Collaborator

@isaachilly isaachilly commented May 20, 2026

I have JIRA issue created

  • branch and/or PR name(s) includes JIRA ID
  • issue has "Fix version" assigned
  • issue "Status" is set to "In review"
  • PR labels are selected
  • FLP integration tests were ran successful
  • Right-Click Context Menu: Right-clicking a cell now brings up a context menu with four actions: two for additive/reductive filtering, and two field-independent actions ("Clear Filter" and "Copy").
  • Opted to use a context menu over modifying the double-click functionality for two reasons:
    1. It avoids disrupting the likely learnt user behaviour of double-click to immediately open the inspector.
    2. Right-click is an intuitive, immediately reachable, and previously unused way to interact with ILG.
  • Adds tests to validate context menu opening, closing and specific action functionalities.

@isaachilly isaachilly self-assigned this May 20, 2026
@isaachilly isaachilly added Frontend InfoLogger javascript Pull requests that update javascript code labels May 20, 2026
@isaachilly isaachilly marked this pull request as ready for review May 20, 2026 15:01
@isaachilly isaachilly requested a review from graduta as a code owner May 20, 2026 15:01
Basic admin refactoring, clamp menu position better (no negative coords, simplify component HTML, add error handling for clipboard.
Remove index-based menu button selectors, mock window.confirm to prevent query confirmation blocking tests, separate click actions from assertions, add clipboard error test.
Copy link
Copy Markdown
Member

@graduta graduta left a comment

Choose a reason for hiding this comment

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

Very nice context menu, good job!
Please see below a few comments and suggestions based on existing use and behaviour of ILG:

  • I believe it would be good to display a hover pop-up when hovering that informs the user to right-click for more options
  • the right click menu should also have the option to open the inspector
  • match and exclude should append instead of replacing the filter. Currently Infologger for all fields except date accept:
    • for one world filters: multi-search by space separation. e.g. system box can accept "ECS GUI" and will search for OR
    • message filter (match/exclude) accepts searching by long messages and the OR is applied on new line separator
    • if we simply "replace" rather than append, shifters will lose existing filters and we do not want that as we give them a pre-defined configuration most of the time with messages to exclude for example
  • severity and label columns should also update the toggle filter buttons

Comment thread InfoLogger/public/log/cellContextMenu.js Outdated
Comment thread InfoLogger/public/log/cellContextMenu.js Outdated
Comment thread InfoLogger/public/log/cellContextMenu.js Outdated
Comment thread InfoLogger/public/log/tableLogsContent.js Outdated
Comment thread InfoLogger/public/log/tableLogsContent.js Outdated
Comment thread InfoLogger/public/log/tableLogsContent.js Outdated
Comment thread InfoLogger/public/log/cellContextMenu.js
Comment thread InfoLogger/public/log/Log.js Outdated
Comment thread InfoLogger/public/log/cellContextMenu.js
isaachilly added 10 commits May 21, 2026 15:54
Caveat: I added colours as classes at the same time and hard to add both so this commit on it's own is incomplete see next.
Caveat: Wrapped Severity and Level with context menu as was doing at the same time so commit it not complete without next.
As well as making "Clear filter" buttons be disabled when there is nothing to clear for that filter.
Additionally refactor into utils and own suite.
Right-clicking a table cell now selects the row as well as
opening the context menu as it is disorienting it doing otherwise.

Since the row is now already selected by the time any menu action runs, the row parameter chain was removed.

Adds a test that verifies this functionality and simplifies "Open Inspector" tests.
Move context menu state and methods from Log into a dedicated model and update all calls.
@isaachilly
Copy link
Copy Markdown
Collaborator Author

isaachilly commented May 21, 2026

Very nice context menu, good job! Please see below a few comments and suggestions based on existing use and behaviour of ILG:

* I believe it would be good to display a hover pop-up when hovering that informs the user to right-click for more options

* the right click menu should also have the option to open the inspector

* match and exclude should append instead of replacing the filter. Currently Infologger for all fields except date accept:
  
  * for one world filters: multi-search by space separation. e.g. system box can accept "ECS GUI" and will search for OR
  * message filter (match/exclude) accepts searching by long messages and the OR is applied on new line separator
  * if we simply "replace" rather than append, shifters will lose existing filters and we do not want that as we give them a pre-defined configuration most of the time with messages to exclude for example

* severity and label columns should also update the toggle filter buttons
  • What do you think about the level filtering? I've set it to have the options of either set level to the next lowest/highest mode.
  • See my reply about the fixed positioning comment.
  • I have not yet looked at PopoverEngine. When I started the ticket I specifically looked at similar CSS-wise components but they didn't map so well hence I wrote my some inspired purpose-built classes instead.
  • Everything else mentioned I added as I understood.

Changes the labels in cellContextMenu.
Changes the names in the affected tests and test utils.
…ith-it' of github.com:AliceO2Group/WebUi into feature/ILG/OGUI-1453/Double-click-on-value-to-filter-with-it
…enContextMenu

It used to be that in the case that two following tests clicked on cells with different field types, you could use the labels to determine if the context menu had rerendered or not.

This is a janky solution and it is easier to rely just on a small timeout in the openContextMenu helper instead.

Thus I am removing these helpers.
@isaachilly isaachilly requested a review from graduta May 21, 2026 16:08
Copy link
Copy Markdown
Member

@graduta graduta left a comment

Choose a reason for hiding this comment

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

Very nice progress! I believe there are two more issues that need addressing and then PR will be good to be shipped.

  1. I did not see a reply or implementation to my suggestion on hover of cell so that the users are aware of the feature
  2. It seems that a small bug is introduced where the separator line between the message column and one before is not there anymore (see screenshot)
Image

Adds a hint button inside each non-empty cell as a suggestion to the user that there are menu options available.
…nings

Update menu height as it was not at all correct and legacy left unchecked.

Accounts for the footer/status-bar so that is always visible.
Add context menu hint tests covering rendering, click behaviour and hover visibility.

Refactor filled and empty row adding.
@isaachilly isaachilly requested a review from graduta May 22, 2026 11:29
@isaachilly isaachilly merged commit ff6851a into dev May 22, 2026
10 checks passed
@isaachilly isaachilly deleted the feature/ILG/OGUI-1453/Double-click-on-value-to-filter-with-it branch May 22, 2026 12:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Frontend InfoLogger javascript Pull requests that update javascript code

Development

Successfully merging this pull request may close these issues.

2 participants