Skip to content

UI: Facet View Enhancement#2336

Merged
tito12 merged 20 commits into
mainfrom
jsonview
Feb 16, 2023
Merged

UI: Facet View Enhancement#2336
tito12 merged 20 commits into
mainfrom
jsonview

Conversation

@tito12

@tito12 tito12 commented Dec 22, 2022

Copy link
Copy Markdown
Contributor

Problem

Now facets are just simple printed JSON, without ability to search, navigate and click on links.
Related to task #2165

Solution

Created dynamic component basen on forked solution and extended, with ability to search through json and navigate easy, expand data and ability to click links.

image

Checklist

  • You've signed-off your work
  • Your changes are accompanied by tests (if relevant)
  • Your change contains a small diff and is self-contained
  • You've updated any relevant documentation (if relevant)
  • You've updated the CHANGELOG.md with details about your change under the "Unreleased" section (if relevant, depending on the change, this may not be necessary)
  • You've versioned your .sql database schema migration according to Flyway's naming convention (if relevant)
  • You've included a header in any source code files (if relevant)

…rmation about license

Signed-off-by: tito12 <vladyslav.sedenko@gmail.com>
Signed-off-by: tito12 <vladyslav.sedenko@gmail.com>
Signed-off-by: tito12 <vladyslav.sedenko@gmail.com>
Signed-off-by: tito12 <vladyslav.sedenko@gmail.com>
Signed-off-by: tito12 <vladyslav.sedenko@gmail.com>
Signed-off-by: tito12 <vladyslav.sedenko@gmail.com>
@wslulciuc

Copy link
Copy Markdown
Member

Created dynamic component basen on forked solution and extended

@tito12, can you provide some background as to why we are forking react-json-view?

@tito12

tito12 commented Jan 2, 2023

Copy link
Copy Markdown
Contributor Author

Created dynamic component basen on forked solution and extended

@tito12, can you provide some background as to why we are forking react-json-view?

This package has most of functions which we need but not all, and specifically this one suit better for our requirements. There are no solution with all of these functionality and it not make sense to create solution like this package from scratch.

@phixMe

phixMe commented Jan 3, 2023

Copy link
Copy Markdown
Member

I'd really prefer a solution here that doesn't fork a repo. What does this look like if we just use the library out of the box entirely?

@tito12

tito12 commented Jan 4, 2023

Copy link
Copy Markdown
Contributor Author

I'd really prefer a solution here that doesn't fork a repo. What does this look like if we just use the library out of the box entirely?

This package not contain search functionality and clickable links, which is important due task requirments.
One if idea is to use patch-package, maybe it could be more clean from code side, what do you think?

tito12 added 4 commits January 9, 2023 19:12
Signed-off-by: tito12 <vladyslav.sedenko@gmail.com>
Signed-off-by: tito12 <vladyslav.sedenko@gmail.com>
Signed-off-by: tito12 <vladyslav.sedenko@gmail.com>
… patch-package unused logic

Signed-off-by: tito12 <vladyslav.sedenko@gmail.com>
@tito12

tito12 commented Feb 7, 2023

Copy link
Copy Markdown
Contributor Author

@phixMe please take a look, I linked github repo with fork as we discussed. Thanks!

Signed-off-by: tito12 <vladyslav.sedenko@gmail.com>
Signed-off-by: tito12 <vladyslav.sedenko@gmail.com>
Signed-off-by: tito12 <vladyslav.sedenko@gmail.com>
Signed-off-by: tito12 <vladyslav.sedenko@gmail.com>
Signed-off-by: tito12 <vladyslav.sedenko@gmail.com>
Signed-off-by: tito12 <vladyslav.sedenko@gmail.com>
@codecov

codecov Bot commented Feb 9, 2023

Copy link
Copy Markdown

Codecov Report

Merging #2336 (865e221) into main (0e434d5) will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff            @@
##               main    #2336   +/-   ##
=========================================
  Coverage     77.11%   77.11%           
  Complexity     1234     1234           
=========================================
  Files           228      228           
  Lines          5572     5572           
  Branches        447      447           
=========================================
  Hits           4297     4297           
  Misses          775      775           
  Partials        500      500           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@tito12

tito12 commented Feb 9, 2023

Copy link
Copy Markdown
Contributor Author

@phixMe please review :)

@phixMe phixMe left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

A few comments here.

Comment thread web/.eslintignore
Comment thread web/Dockerfile Outdated
Comment thread web/src/components/jobs/RunInfo.tsx
Signed-off-by: tito12 <vladyslav.sedenko@gmail.com>
Signed-off-by: tito12 <vladyslav.sedenko@gmail.com>

@phixMe phixMe left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks, this looks amazing.

Signed-off-by: tito12 <vladyslav.sedenko@gmail.com>
@tito12 tito12 merged commit 56fa342 into main Feb 16, 2023
@tito12 tito12 deleted the jsonview branch February 16, 2023 13:38
jonathanpmoraes referenced this pull request in nubank/NuMarquez Feb 6, 2025
* Json View component, forked package react-json-view and changes, information about license

Signed-off-by: tito12 <vladyslav.sedenko@gmail.com>

* fix config jest test env

Signed-off-by: tito12 <vladyslav.sedenko@gmail.com>

* jsonView component for jobs page

Signed-off-by: tito12 <vladyslav.sedenko@gmail.com>

* fix eslint for external modules

Signed-off-by: tito12 <vladyslav.sedenko@gmail.com>

* remove empty line

Signed-off-by: tito12 <vladyslav.sedenko@gmail.com>

* Move to patch-package solution and remove module code

Signed-off-by: tito12 <vladyslav.sedenko@gmail.com>

* Remove alias for package in webpack

Signed-off-by: tito12 <vladyslav.sedenko@gmail.com>

* Eslint fix

Signed-off-by: tito12 <vladyslav.sedenko@gmail.com>

* Link repo with package searchable-react-json-view from github. Remove patch-package unused logic

Signed-off-by: tito12 <vladyslav.sedenko@gmail.com>

* Remove unused code

Signed-off-by: tito12 <vladyslav.sedenko@gmail.com>

* Remove unused code

Signed-off-by: tito12 <vladyslav.sedenko@gmail.com>

* Add git to docker to be able to download npm package from github

Signed-off-by: tito12 <vladyslav.sedenko@gmail.com>

* Revert "Add git to docker to be able to download npm package from github"

This reverts commit 8c5eab23aa870a187a594510492e6dcd86284a56.

* Change new package url from git style to simple link

Signed-off-by: tito12 <vladyslav.sedenko@gmail.com>

* Add git to docker to be able to download npm package from github

Signed-off-by: tito12 <vladyslav.sedenko@gmail.com>

* Remove unused code. Move all json view to new component

Signed-off-by: tito12 <vladyslav.sedenko@gmail.com>

* Revert file eslintignore

Signed-off-by: tito12 <vladyslav.sedenko@gmail.com>

---------

Signed-off-by: tito12 <vladyslav.sedenko@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants