-
-
Notifications
You must be signed in to change notification settings - Fork 93
feat: add support for UI customization #798
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
base: main
Are you sure you want to change the base?
Conversation
Use flex replace absolute.
- revert setting form labelCol - add theme editor link
Hi Joanas apologies for the late review, thanks for the interest in contributing. At a first pass this looks great. Looking through the changes I see you've updated the protobuf builds but I don't think this CL strictly depends on updated definitions & the updated go versions might also be best split into a separate CL. Would you mind dropping the changes to protubuf / go mod or splitting these into a separate PR? With that I think it looks good to go ahead and merge! |
That's OK ~ At the beginning, I just wanted to modify the font style. So I directly modified the code and submitted the PR. However, later I found that I could directly output the configuration through the antd theme editor and apply it directly to the code. So I thought about making a UI interface configuration so that users could control these appearances by themselves, including whether to use the compact style. But after I added these functions, I didn't modify the title and content of this PR in a timely manner. Because I want to save these configurations to the backend, I added two configuration items in config.proto. This is my first time to come into contact with technologies related to protobuf. I used buf to regenerate. Since it was the first time I used it, I didn't specify the version when installing buf and @bufbuild/protoc-gen-es, resulting in using the latest version 2.5.2. And I'm not sure why the order of the fields in the generated pb.go and pb.ts is different from the previous ones. If you can help me answer this question, I would really appreciate it. Regarding "go mod", it's because when I did
I referred to here and changed it to 1.24.0. So, do you think the following is okay :
|
Ah, thanks for the extra detail-- in my initial skimming I somehow completely missed that there were golang changes included here, understanding that you're adding fields to the config this makes sense to me. And the theme editing is honestly pretty neat. Had a chance this evening to try running the PR and I think it looks great, the ability to switch between compact/non-compact and the ability to enter a custom theme are both quite neat. The only caveats to all of this I'd add is that it's a bit awkward that the settings are global to all users, but I think multiuser installs are relatively uncommon enough that this is fine. The alternative implementation would be storing preferences in browser local storage, but it's probably more annoying than not to need to customize to each device.
I'll go ahead and regenerate with the version of protobuf I've been using, in general I need to work on the contributor tooling to make the protobuf builds reproducible with pinned versions of the toolchains.
Specifically re: 1.24.0 vs 1.24 -- a bit surprised I've not run into this, it looks like we are now expected to pin a patch version in the module. I think I'm fine with this change. |
I'll aim to go ahead and merge this PR, just want to add on that the time taken on the contribution is really appreciate, thanks for for taking the time to refine the UI code across the board. The new theming is a very neat feature and the drive-by improvements on the styling / React code are appreciated! Looks like there are a few merge conflicts that need resolving, if you have some time to sync up I'll go ahead and merge this-- and I'll aim to wait for this to get in before merging #815 :) |
810daa4
to
5bc63d4
Compare
Hello @garethgeorge , sorry for taking so long to resovle this conflicts, i resolved conflicts and i saw that you modified the style of the setting modal, and i made the same modification for new ui setting as well. |
.goreleaser.yaml
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, looks like something's gotten confused in the merge. Can you try flattening your change history with git rebase
and replying it on top of the latest main?
Looks like a bunch of the multihost sync changes are showing up as diffs in this cl when I'm reviewing.
No problem -- thanks for taking a pass on the merge conflicts and sorry they're a pain. It looks like something has gotten a bit confused, I think the easiest thing may be to use rebase to flatten your commit history and replay it onto main as a single commit, some force pushing might be involved on your end. Deferring to your judgement / what's easiest though :). It looks like I'm seeing unrelated changes in the history for this review. |
Features:
About ant theme editor : here
For example "change font size and border radius":

Copy config:

Then paste to settings "ant design theme tokens".