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

UI - Add filter for tables #1862

Merged
merged 3 commits into from
Jan 25, 2024

Conversation

thfries
Copy link
Contributor

@thfries thfries commented Jan 14, 2024

Hello @thjaeckle,
I again totally messed up the other PR, so please find this new PR for #1818.
Merge conflict should be resolved. Headers to new files added.

As mentioned, if this works well, I would like to extend that to RQL and use it also for Things search.
Any feedback also well come. I expect there can be improvements.

Details on commits:

  • introduce jest for unit testing
  • created new tableFilter web component
  • upgraded dependencies
  • fixed error in tsconfig
  • introduced basic filter to create queries in UI
  • dropdown entries now can attach data
  • split adding dropdown entries for single and multiple entries
  • moved utility methods to utils
  • added table filter to connection logs
  • added table filter to incoming thing messages
  • connection metrix now shows red numbers for failures
  • removed unused symbol in Things tab

Resolves: #1818

- introduce jest for unit testing
- created new tableFilter web component
- upgraded dependencies
- fixed error in tsconfig
- introduced basic filter to create queries in UI
- dropdown entries now can attach data
- split adding dropdown entries for single and multiple entries
- moved utility methods to utils
- added table filter to connection logs
- added table filter to incoming thing messages
- connection metrix now shows red numbers for failures
- removed unused symbol in Things tab

Signed-off-by: thfries <thomas.fries0@gmail.com>
@thjaeckle thjaeckle added the UI Issues related to the Ditto explorer UI label Jan 14, 2024
@thjaeckle thjaeckle added this to the 3.5.0 milestone Jan 14, 2024
@thjaeckle
Copy link
Member

Thanks a lot @thfries
Will have a look, looking forward to new features in the UI 😍

@thjaeckle
Copy link
Member

@thfries I had a look at the added filter

  • for connections logs, this is already very useful, great addition 👍
  • for thing events (incoming thing messages) it depends on the kind of events
    • I think it works well for "modify" events, where we always have a path which changed
    • but it does not provide much benefit when a merge is done on / path and only the "Field(s)" column contains which features e.g. were modified in that merge event
      • maybe this can be solved by a custom JsonPath, but I did not immediately figure it out :D
      • nothing which would have to be resolved as part of this PR - just wanted to point out that when using mainly/only merge on root level (/ path), the "basic filter" does not provide much value

Some other remarks:

  • I think the toggle would need a label to make clear that it switches between basic filter and JsonPath mode
  • The filter input field suggests to autocomplete my stored credentials for the website - I once already fixed that for the things search in commit 2370356 - maybe you could add that as well for this new input?
    • that would probably make sense for all non-auth* input fields of the UI
  • Would providing additional JsonPath expressions make sense to add via the environment? Then e.g. a "combined" expression could be also saved / used - or even a complete custom JsonPath expression
    • I assume the "star" button for making a favorite would then needed to be added

And regarding the RQL based filtering:

  • it would be IMO way better to provide an RQL statement to the "filter" for the "Incoming Thing messages" - as the backend also does support backend-side filtering - and now even the "history API" of Ditto supports RQL based filters as well
    • this would make the "story" of how filtering in Ditto is done "complete", if the UI would also use the same filter to do either browser-side or even server-side filtering
    • maybe this could be an addition - and to then select between JsonPath and RQL mode to apply
      • could be also done in a future PR
  • for connection logs however, the JsonPath approach is a great choice 👍 - RQL would not make sense here

Do you have an idea for the mentioned remarks?
I think we could also accept and merge the PR and improve some things later .. but maybe you see some "low hanging" fruits.

@thfries
Copy link
Contributor Author

thfries commented Jan 19, 2024

Hi @thjaeckle,

thanks for your feedback. Highly appreciated.
I would like to consider some of the discussed improvements in this PR. Is there a tight deadline for 3.5?

  • I'll check how you could filter merges to "/" path. I m thinking about searching in _context/value
  • I was already struggling with the label for the advanced toggle, because of missing space. I was hoping that you can cover most cases with "basic" filters and you need the advanced mode only in rare exceptional cases...
  • The input autocomplete behavior should be fixed in any case
  • Yes, adding the star and storing filters was something I had in my mind, too. I would even also store the basic filters and provide option to give a name to a filter. (btw: one crazy idea was, to store the environment in a thing so you can share your filters and fields etc. with the rest of the team... 🤪)
  • do you have an idea how to process RQL filtering on the browser side? my assumption was that any use of RQL involves the ditto server?

So I will investigate to those improvements...

@thjaeckle
Copy link
Member

Ditto 3.5 was planned to be released next week.
So too big changes to the PR are probably unrealistic.

So I would eg not try to cover the rql filtering.
RQL filtering on client side could be possible, but I don't think we should invest in that. JsonPath is mightier.
Maybe instead we can switch between server side and client side filtering at a later point.

@thjaeckle
Copy link
Member

@thfries If you don't mind, I would like to work on fixing the one point - so that we can get this PR ready to be merged for the Ditto 3.5 release:

The filter input field suggests to autocomplete my stored credentials for the website

I already did that for the other search bar, so should not be a big thing..

… fields

Signed-off-by: Thomas Jäckle <thomas.jaeckle@beyonnex.io>
Copy link
Member

@thjaeckle thjaeckle left a comment

Choose a reason for hiding this comment

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

I added this little fix - preventing the browser from suggesting credentials for the input field. And also for the policyId.

From my point of view good to be merged, as we can tackle the other mentioned issues as follow-ups.

@thjaeckle thjaeckle merged commit 2042b6f into eclipse-ditto:master Jan 25, 2024
3 checks passed
@thfries
Copy link
Contributor Author

thfries commented Jan 26, 2024

Hi @thjaeckle, thank you for stepping in and considering the PR for 3.5! I did not find the time. Happy to have that in and thank you for the fix.

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
UI Issues related to the Ditto explorer UI
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

UI: Add search filter option to incoming messages and connection logs
2 participants