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

Updates to dev UI #629

Open
wants to merge 18 commits into
base: main
Choose a base branch
from
Open

Conversation

javisperez
Copy link
Contributor

This PR updates the Dev Mode UI to a new look. With changes in the UX, the flow of the data and useful snippets for Python, Node and SH.

Copy link
Contributor

@amisevsk amisevsk left a comment

Choose a reason for hiding this comment

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

Is there a good way to test this locally with a running kit dev server? I tried pnpm dev but the conversation hangs, and kit dev is still serving the old one.

Copy link
Contributor

Choose a reason for hiding this comment

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

Not related to this file, but the .nvmrc in frontend/dev-mode is still on v18.

... also I wasn't able to build it locally using v20 or v22 (pnpm i was failing)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ill update the nvmrc thanks for the catch. About running it locally, the way i do it to test it is that i run kit unpack to get a kitfile + model and then do:

kit dev start ./unpacked --port 64246

and then in the frontend/dev-mode/src/services/completion.ts file i update the apiUrl const and point it to http://localhost:64246 (change that i dont commit btw), and

pnpm dev

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh btw i have kit install via homebrew because is easier, so i just do kit without using go or anything.

@amisevsk
Copy link
Contributor

amisevsk commented Nov 27, 2024

On firefox, it looks like there's a discrepancy in text size between the code block on the right and the text on the left (14px vs 18px), which looks a little weird to me:

image

C.f. 14px code with a 1.5 line height (I just edited the page locally):

image

edit: this is on my 1440p monitor

@gorkem
Copy link
Contributor

gorkem commented Nov 27, 2024

This is what I got when I tried it.
I have built the UI with

go generate ./...

Are there any build changes that we should reflect to the generate script?

Screenshot 2024-11-27 at 5 49 43 PM

@javisperez
Copy link
Contributor Author

@amisevsk the font size discrepancy i think is expected, I have reused the same code snippets component from the other sites (kitops docs and jozu hub deployment tab) so the snippet style should come from that component, hence the background color, while the left side is just the figma design. I can update the font to be the same on both sites but i'm just trying to avoid having to update the snippet again because "its not the same as kitops". Any thoughts?

@javisperez
Copy link
Contributor Author

This is what I got when I tried it. I have built the UI with

go generate ./...

Are there any build changes that we should reflect to the generate script?

Screenshot 2024-11-27 at 5 49 43 PM

@gorkem i dont understand that question :D what is the generate script ? is that for me or Angel?

also, why are your scrollbars so weird looking? that looks off... ill try to reproduce..

@gorkem
Copy link
Contributor

gorkem commented Nov 28, 2024

@javisperez when we build and package the UI as part of the package it uses https://github.com/javisperez/kitops/blob/8b0364757e5e18dd329022f08e43c9aa38a12605/pkg/lib/harness/generated/gen_common.sh#L6-L15 Can you check if this looks correct?

@javisperez
Copy link
Contributor Author

@gorkem @amisevsk things should be way better now, feel free to pull and try again please :D.

@amisevsk
Copy link
Contributor

I totally forgot about go generate :D

We should add it to the frontend readme

@gorkem
Copy link
Contributor

gorkem commented Nov 28, 2024

The build issues are gone for me... But I still see the ugly scrollbars on the code section. It feels like the font for the code is disproportionately larger with the rest of the UI.

Also, should the code be highlighted?

image

Copy link
Contributor

@amisevsk amisevsk left a comment

Choose a reason for hiding this comment

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

Tested on firefox and chrome (Linux) and it looks good to me. I can't reproduce the scrollbars issue Gorkem mentioned.

The "Response" panel on the landing page doesn't really make sense to me though:

image

@javisperez
Copy link
Contributor Author

@amisevsk that looks like a bug :flip-the-table: on it

@javisperez
Copy link
Contributor Author

@gorkem @amisevsk wanna give it another try? i cant reproduce the scrollbar issues but i did fix a few things with the highlight js dependency, hopefully that will fix it.

Copy link
Contributor

@amisevsk amisevsk left a comment

Choose a reason for hiding this comment

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

Tested latest changes with pnpm preview and the syntax highlighting is working. On Linux, I'm seeing normal (small) scroll bars.

@javisperez
Copy link
Contributor Author

@gorkem @amisevsk added custom scrollbar styles that should be consistent across devices (trackpad vs mouse) feel free to try again :D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants