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.

Sign up for GitHub

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

Jump to bottom

Refresh UI and modernize its dependencies #22831

Merged
merged 9 commits into from
Oct 9, 2024
Merged

Refresh UI and modernize its dependencies #22831

merged 9 commits into from
Oct 9, 2024

Conversation

wendigo
Copy link
Contributor

@wendigo wendigo commented Jul 26, 2024 •
edited
Loading

Some small touches to make UI a little bit better :)

Added a sql highlighting to a list of queries with the changed theme that is more subtle than the previous one:

Screenshot 2024-07-26 at 16 52 23 Screenshot 2024-07-26 at 17 14 59

Updates:

  1. vis.js timeline to 7.7.3
  2. clipboard.js to 2.0.11
  3. highlight.js to 11.10.0
  4. JQuery to 3.7.1

Removes D3 as it was only used to download JSON (and JQuery does that too)

@cla-bot cla-bot bot added the cla-signed label Jul 26, 2024
@wendigo wendigo requested a review from krvikash July 26, 2024 14:53
@github-actions github-actions bot added the ui Web UI label Jul 26, 2024
@wendigo wendigo requested a review from mosabua July 26, 2024 14:54
@wendigo wendigo force-pushed the serafin/update-ui branch from c0de132 to f63bd51 Compare July 26, 2024 15:00
@wendigo wendigo requested a review from electrum July 26, 2024 15:05
@wendigo wendigo mentioned this pull request Jul 26, 2024
Closed
@wendigo wendigo force-pushed the serafin/update-ui branch from f63bd51 to 025b8e2 Compare July 26, 2024 15:50
@wendigo
Copy link
Contributor Author

wendigo commented Jul 26, 2024

Screenshot 2024-07-26 at 17 50 59

@wendigo
Copy link
Contributor Author

wendigo commented Jul 26, 2024

I've added links that allows to go directly to query json, live plan and stage performance

@wendigo wendigo force-pushed the serafin/update-ui branch from 025b8e2 to 7470810 Compare July 26, 2024 16:25
@wendigo wendigo requested a review from martint July 26, 2024 22:03
@wendigo wendigo force-pushed the serafin/update-ui branch from 7470810 to e34705d Compare July 29, 2024 17:22
@wendigo
Copy link
Contributor Author

wendigo commented Jul 29, 2024

@electrum @mosabua PTAL

@wendigo
Copy link
Contributor Author

wendigo commented Jul 29, 2024

Screenshot 2024-07-29 at 19 34 19 Screenshot 2024-07-29 at 19 34 10

@mosabua
Copy link
Member

mosabua commented Jul 29, 2024

I thought we got rid of having the vendor folder with the binaries checked in? Not vis is in there

@wendigo
Copy link
Contributor Author

wendigo commented Jul 29, 2024

@mosabua vendor folder is still there. We will remove it once we move some of the features to jsx

mosabua
mosabua reviewed Jul 29, 2024
View reviewed changes
Copy link
Member

@mosabua mosabua left a comment

Choose a reason for hiding this comment

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

Couple of questions mostly. Overall definitely a great little upgrade.

core/trino-main/src/main/resources/webapp/assets/js/timeline.js Outdated Show resolved Hide resolved
core/trino-main/src/main/java/io/trino/server/ui/UiQueryResource.java Outdated Show resolved Hide resolved
core/trino-main/src/main/java/io/trino/server/ui/UiQueryResource.java Outdated Show resolved Hide resolved
core/trino-main/src/main/resources/webapp/assets/js/timeline.js Outdated Show resolved Hide resolved
core/trino-main/src/main/resources/webapp/disabled.html Outdated Show resolved Hide resolved
core/trino-main/src/main/resources/webapp/embedded_plan.html Outdated Show resolved Hide resolved
core/trino-main/src/main/resources/webapp/src/components/QueryHeader.jsx Outdated Show resolved Hide resolved
core/trino-main/src/main/resources/webapp/src/components/QueryList.jsx Outdated Show resolved Hide resolved
core/trino-main/src/main/resources/webapp/src/components/QueryList.jsx Outdated Show resolved Hide resolved
core/trino-main/src/main/resources/webapp/stage.html Outdated Show resolved Hide resolved
@wendigo wendigo force-pushed the serafin/update-ui branch from e34705d to 3d2001e Compare July 31, 2024 18:40
@wendigo
Copy link
Contributor Author

wendigo commented Jul 31, 2024

Resolved conflicts

@github-actions GitHub Actions
Copy link

This pull request has gone a while without any activity. Tagging the Trino developer relations team: @bitsondatadev @colebow @mosabua

@github-actions github-actions bot added the stale label Aug 22, 2024
@wendigo wendigo requested a review from mosabua September 11, 2024 19:21
mosabua
mosabua reviewed Sep 11, 2024
View reviewed changes
Copy link
Member

@mosabua mosabua left a comment

Choose a reason for hiding this comment

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

A couple of questions and suggestions, nothing major, very close

core/trino-web-ui/src/main/resources/webapp/timeline.html Show resolved Hide resolved
core/trino-web-ui/src/main/resources/webapp/vendor/vis/dist/vis.css Show resolved Hide resolved
core/trino-web-ui/src/main/resources/webapp/vendor/vis/vis.css Show resolved Hide resolved
core/trino-web-ui/src/main/resources/webapp/vendor/vis/vis.css
.vis.timeline .vispanel.right,
.vis.timeline .vispanel.top,
.vis.timeline .vispanel.bottom {
border: 1px #bfbfbf;
Copy link
Member

Choose a reason for hiding this comment

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

Did you end up creating a colour palette and using those?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no, can you share the Trino pallette so I can use it here?

core/trino-web-ui/src/main/resources/webapp/vendor/vis/vis.css Show resolved Hide resolved
core/trino-web-ui/src/main/resources/webapp/vendor/vis/vis.css Show resolved Hide resolved
core/trino-web-ui/src/main/resources/webapp/vendor/vis/vis.css Show resolved Hide resolved
core/trino-web-ui/src/main/resources/webapp/vendor/vis/vis.css Show resolved Hide resolved
core/trino-web-ui/src/main/resources/webapp/src/components/QueryList.jsx Show resolved Hide resolved
core/trino-web-ui/src/main/resources/webapp/src/components/QueryHeader.jsx Outdated Show resolved Hide resolved
@mosabua
Copy link
Member

mosabua commented Sep 11, 2024

Please also suggest a release note entry since there is a new screen with the explain plan

@github-actions github-actions bot removed the stale label Sep 12, 2024
@github-actions GitHub Actions
Copy link

github-actions bot commented Oct 4, 2024

This pull request has gone a while without any activity. Tagging the Trino developer relations team: @bitsondatadev @colebow @mosabua

@github-actions github-actions bot added the stale label Oct 4, 2024
@mosabua
Copy link
Member

mosabua commented Oct 4, 2024

This should be good after a final check from @wendigo

@wendigo
Copy link
Contributor Author

wendigo commented Oct 4, 2024

@mosabua I'll go back to it next week

@github-actions github-actions bot removed the stale label Oct 7, 2024
@wendigo wendigo force-pushed the serafin/update-ui branch from 0f45766 to 4d36e5c Compare October 9, 2024 16:05
mosabua
mosabua approved these changes Oct 9, 2024
View reviewed changes
Copy link
Member

@mosabua mosabua left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Looks good to me now. Lets ship it after CI passes (apart from any false alarms to ignore) and you did a final local test..

@wendigo
Copy link
Contributor Author

wendigo commented Oct 9, 2024 •
edited
Loading

I've updated colors to use Trino palette:

Screenshot 2024-10-09 at 19 01 05 Screenshot 2024-10-09 at 19 03 12

@mosabua wdyt?

@wendigo wendigo force-pushed the serafin/update-ui branch from 0179713 to f7f9f92 Compare October 9, 2024 17:03
@mosabua
Copy link
Member

mosabua commented Oct 9, 2024

That looks great @wendigo .. I would go with that. Hopefully we can refine the colour selection with some shades over time .. but this is good as it is now.

@wendigo
Copy link
Contributor Author

wendigo commented Oct 9, 2024

@mosabua yeah, we need more colors :)

@wendigo
Copy link
Contributor Author

wendigo commented Oct 9, 2024

Screenshot 2024-10-09 at 19 06 22

Additional icons on the query list that allows to jump directly to some of the tabs

@wendigo wendigo merged commit aabf405 into master Oct 9, 2024
111 checks passed
@wendigo wendigo deleted the serafin/update-ui branch October 9, 2024 18:29
@github-actions github-actions bot added this to the 461 milestone Oct 9, 2024
@mosabua mosabua mentioned this pull request Oct 9, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mosabua mosabua mosabua approved these changes

@krvikash krvikash Awaiting requested review from krvikash

@electrum electrum Awaiting requested review from electrum

@martint martint Awaiting requested review from martint

Assignees
No one assigned
Labels
cla-signed ui Web UI
Milestone
461
Development

Successfully merging this pull request may close these issues.
2 participants
@wendigo @mosabua