Webapp Preview: Core components #22869

koszti · 2024-07-29T23:57:29Z

Description

Core components and layout for the preview UI. Partially addressing and proposing technical solutions for #22697

It uses Material UI and features a look and feel similar to the Trino website and documentation, including a comparable color palette. The code also draws inspiration from the ideas and structure of Trino Gateway.

Additional context and related issues

Components added by this PR:

Basic layout with header, drawer and main content
Styles aligned with the trino website and documentation (theme.tsx)
Basic router with navigation to placeholder pages with themed Material UI sample components
State manager - persisting local config. (No authn/authz in this PR for now)
Light/Dark theme switcher
Error message snackbar (toast)

Screenshots:

Light Theme:

Dark Theme:

Snackbar (failure message example):

Release notes

(x) This is not user-visible or is docs only, and no release notes are required.
( ) Release notes are required. Please propose a release note for me.
( ) Release notes are required, with the following suggested text:

koszti · 2024-07-30T00:19:56Z

@mosabua , @wendigo, I can keep working on the preview UI and this is the next item in the pipeline. Are you happy if I keep sending relatively small PRs like this one, and by doing that slowly addressing the items in the list in #22697?

mosabua · 2024-07-30T05:55:35Z

@mosabua , @wendigo, I can keep working on the preview UI and this is the next item in the pipeline. Are you happy if I keep sending relatively small PRs like this one, and by doing that slowly addressing the items in the list in #22697?

Yes.. however I think ideally we start reviewing and merging. We need to get some consensus with other maintainers about the approach and such as well. We should maybe discuss this all some more in either in core-dev channel.

wendigo · 2024-07-31T14:29:43Z

@mosabua my quality bar is pretty old at the moment for the preview UI. We should just skim PR and go ahead. There is no point in length discussions imo

mosabua · 2024-07-31T16:02:10Z

Agreed @wendigo .. I just want to make sure it is tucked away safely and doesnt impact the rest of Trino negatively. If that is the case I am happy to just merge rather rapidly all under the understanding that this is a preview and very much under construction.

mosabua · 2024-07-31T16:06:49Z

I think what I am saying is that we need some sort of config that completely disables the new UI from being used.. it can still be in the binary of course but it should be impossible to use it. And users have to switch the property to true for the preview to work.

Something like

web-ui.preview-enabled=true

or maybe even create a namespace in the config in case we need more of them.

web-ui.preview.enabled=true

https://trino.io/docs/current/admin/properties-web-interface.html

Does that make sense @koszti and @wendigo ?

Update: I see that this is already in place from #22783 .. we should add the config to the docs I think. And yes.. lets ship this.

wendigo · 2024-07-31T16:26:53Z

@mosabua this is already it. The new web-preview is disabled by default and you need to switch on config property :)

wendigo · 2024-07-31T16:27:08Z

@mosabua we should get #22892 first. This is something I've discussed with David

mosabua · 2024-07-31T16:32:57Z

@mosabua we should get #22892 first. This is something I've discussed with David

That sounds good.

mosabua · 2024-07-31T16:43:59Z

If we get the new separate web ui module in .. do we use that same module for the new preview UI .. I assume yes .. but I could also be convinced that having a separate module for the new UI is even cleaner.

wendigo · 2024-07-31T17:13:01Z

@mosabua new preview UI is already merged (at least scaffolding) and it's moved to trino-web-ui module too :)

mosabua · 2024-07-31T17:32:53Z

So we still with same module.. sounds good.

wendigo · 2024-07-31T18:07:02Z

@koszti please rebase - you need to move these files to the core/trino-web-ui module

koszti · 2024-07-31T19:29:53Z

@wendigo rebase done

wendigo · 2024-07-31T19:57:29Z

@koszti I'll review it tommorow

wendigo · 2024-08-01T10:12:22Z

core/trino-web-ui/src/main/resources/webapp-preview/package.json

@@ -11,19 +11,25 @@
    "preview": "vite preview"
  },
  "dependencies": {
+    "@douyinfe/semi-icons": "^2.63.1",


I think that we should rather use https://www.npmjs.com/package/material-symbols

cc @mosabua

let’s not mix items from two different design packages. (semi and material). If you prefer material design then let’s switch not only the icons but the entire lib, otherwise we might end up with a weird and mixed look and feel.

yeah! makes sense :)

@koszti is this a heavy lift to use material ui here?

so material ui + material symbols (as called out here: https://m3.material.io/styles/icons/overview)

wendigo · 2024-08-01T10:17:20Z

core/trino-web-ui/src/main/resources/webapp-preview/src/components/dashboard.tsx

+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+import { Empty } from "@douyinfe/semi-ui";


why not material design? it's more established than semi-ui

cc @mosabua

semi design only because its been used in trino-gateway. But I’m happy to use something else, including material-ui as well as I’m not the greatest fan of semi design :)I leave it you to decide

Me neither. I think we should stick to the material-ui

~~ok I will rework this PR and let you know when it’s ready to review again~~

I am also inclined to go with material but I am not a webdev any more... @colebow has more experience.

The fact that semi is used in the Trino Gateway UI is something we should keep in mind but it should not force us. Ideally we want to be able to reuse components across at some stage .. but we are not there yet.

Also .. if material proofs to be a good choice here.. we can also refactor the Trino Gateway UI if we want.

Also .... we already use material for the website as well. And Trino Gateway docs uses material for mkdocs

Yeah. Material it is

@wendigo @mosabua it seems Material UI is the clear winner. I've updated the PR to use MUI and refreshed the screenshots above. Additionally, I made a minimalistic theme adjustment to align the basic styles with the Trino website and documentation.

That looks nice!

colebow

I'm not sure how much we need all of the snackbar-themed Snackbar, but it's easy enough to remove and I like that I'm learning MaterialUI by reviewing it. This looks good.

colebow · 2024-08-05T17:28:30Z

core/trino-web-ui/src/main/resources/webapp-preview/src/App.tsx

+  );
+}
+
+const NotFound = () => {


It'd be fun to include a Commander Bun Bun image as part of this.

core/trino-web-ui/src/main/resources/webapp-preview/src/App.tsx

core/trino-web-ui/src/main/resources/webapp-preview/src/components/QueryHistory.tsx

wendigo

overall LGTM. I'd add prettier though

koszti · 2024-08-06T12:49:36Z

overall LGTM. I'd add prettier though

@wendigo I added prettier with the same .prettierrc to this PR: #22953 . All files are reformatted.

The package.json now has three targets:

lint: To run eslint enforcing coding standards and patterns, detecting code quality issues, and identifying bugs
prettier:format: To write the formatted files. This comes handy during development
prettier:check: To check if all files are formatted correctly. This can be used to run at PR time by github actions.

Wdyt, is this OK and should we add npm run prettier:check and maybe npm run lint to GHA, if yes then where is the right place?

wendigo · 2024-08-06T13:08:44Z

@koszti we should add probably a separate "npm run check" execution to the pom which will run all checkers (linters, tests if any in the future etc)

koszti · 2024-08-06T16:12:32Z

ok, added a new check script to package.json and a new execute to pom.xml so prettier, eslint and other future checkers will run automatically.

koszti · 2024-08-08T13:34:22Z

Is there anything I can do to help get this merged?I'm asking because I'd like to start working on more PRs that depending on this one.

wendigo · 2024-08-08T13:41:54Z

@koszti I'm gonna review it really soon :)

core/trino-web-ui/src/main/resources/webapp-preview/.prettierrc

wendigo · 2024-08-13T12:41:12Z

I've applied some small fixes and going to merge this soon.

wendigo · 2024-08-13T15:04:59Z

@koszti here you go :)

cla-bot bot added the cla-signed label Jul 29, 2024

koszti force-pushed the webapp-preview-core-components branch from e35e915 to 756330a Compare July 30, 2024 00:00

koszti mentioned this pull request Jul 30, 2024

Modernize the Trino webapp #22697

Open

koszti force-pushed the webapp-preview-core-components branch from 756330a to 62fc1ec Compare July 30, 2024 00:07

koszti force-pushed the webapp-preview-core-components branch from 62fc1ec to b80ce91 Compare July 30, 2024 00:27

mosabua mentioned this pull request Jul 31, 2024

Add docs for Preview Web UI #22900

Merged

koszti force-pushed the webapp-preview-core-components branch from b80ce91 to 905d0dc Compare July 31, 2024 19:27

wendigo reviewed Aug 1, 2024

View reviewed changes

koszti force-pushed the webapp-preview-core-components branch 2 times, most recently from e3ff489 to ba669ae Compare August 4, 2024 22:50

colebow approved these changes Aug 5, 2024

View reviewed changes

koszti force-pushed the webapp-preview-core-components branch from ba669ae to 2f147cc Compare August 5, 2024 21:09

wendigo reviewed Aug 6, 2024

View reviewed changes

core/trino-web-ui/src/main/resources/webapp-preview/src/App.tsx Outdated Show resolved Hide resolved

wendigo reviewed Aug 6, 2024

View reviewed changes

core/trino-web-ui/src/main/resources/webapp-preview/src/components/QueryHistory.tsx Outdated Show resolved Hide resolved

wendigo reviewed Aug 6, 2024

View reviewed changes

koszti force-pushed the webapp-preview-core-components branch from 2f147cc to b2aa67c Compare August 6, 2024 12:42

koszti force-pushed the webapp-preview-core-components branch from b2aa67c to 2505cf1 Compare August 6, 2024 16:08

wendigo reviewed Aug 13, 2024

View reviewed changes

core/trino-web-ui/src/main/resources/webapp-preview/.prettierrc Outdated Show resolved Hide resolved

Webapp Preview: core components

4240906

wendigo force-pushed the webapp-preview-core-components branch from 2505cf1 to 4240906 Compare August 13, 2024 12:40

wendigo approved these changes Aug 13, 2024

View reviewed changes

wendigo merged commit 918af78 into trinodb:master Aug 13, 2024
93 of 94 checks passed

github-actions bot added this to the 454 milestone Aug 13, 2024

koszti deleted the webapp-preview-core-components branch August 13, 2024 16:54

colebow mentioned this pull request Aug 14, 2024

Add Trino 454 release notes #22977

Merged

Webapp Preview: Core components #22869

Webapp Preview: Core components #22869

Conversation

koszti commented Jul 29, 2024 • edited Loading

Description

Additional context and related issues

Screenshots:

Release notes

koszti commented Jul 30, 2024 • edited Loading

mosabua commented Jul 30, 2024 • edited Loading

wendigo commented Jul 31, 2024

mosabua commented Jul 31, 2024

mosabua commented Jul 31, 2024 • edited Loading

wendigo commented Jul 31, 2024

wendigo commented Jul 31, 2024

mosabua commented Jul 31, 2024

mosabua commented Jul 31, 2024

wendigo commented Jul 31, 2024

mosabua commented Jul 31, 2024

wendigo commented Jul 31, 2024

koszti commented Jul 31, 2024

wendigo commented Jul 31, 2024

Choose a reason for hiding this comment

koszti Aug 1, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

koszti Aug 1, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

koszti Aug 1, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mosabua Aug 1, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

colebow left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wendigo left a comment

Choose a reason for hiding this comment

koszti commented Aug 6, 2024 • edited Loading

wendigo commented Aug 6, 2024

koszti commented Aug 6, 2024

koszti commented Aug 8, 2024

wendigo commented Aug 8, 2024

wendigo commented Aug 13, 2024

wendigo commented Aug 13, 2024

koszti commented Jul 29, 2024 •

edited

Loading

koszti commented Jul 30, 2024 •

edited

Loading

mosabua commented Jul 30, 2024 •

edited

Loading

mosabua commented Jul 31, 2024 •

edited

Loading

koszti Aug 1, 2024 •

edited

Loading

koszti Aug 1, 2024 •

edited

Loading

koszti Aug 1, 2024 •

edited

Loading

mosabua Aug 1, 2024 •

edited

Loading

koszti commented Aug 6, 2024 •

edited

Loading