feat: add customization options for the login page #5633
Conversation
|
If none of the parameters are given, everything stays the same.
|
|
app-name also exits on workspace pages. just like page title. I think the customization is not complete.
|
you are right, i will add it |
|
I took look at it and it seems that the title in the tab is set by vscode itself. It shows code-server because that is part of the patched product.json. |
So w can only modify it at the source code and then repackage it. |
yes, that's what I mean. One solution would be to serve the product.json dynamically but I am not willing to implement that and it is way too much overhead and complexity for such a simply feature |
|
if so，the pr no meaning？ we have to modify source code on every project according our customize needs。 |
I think it is still better to be able to customize the login page than nothing. |
jsjoeio
changed the title
|
I think this is okay for us to accept. @niwla23 do you mind adding some tests? |
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #5633 +/- ##
==========================================
+ Coverage 72.52% 72.61% +0.09%
==========================================
Files 30 30
Lines 1678 1680 +2
Branches 366 368 +2
==========================================
+ Hits 1217 1220 +3
+ Misses 398 397 -1
Partials 63 63
Continue to review full report at Codecov.
|
I will add some E2E tests tomorrow |
|
Do you mind doing unit tests instead? I'd rather not increase our e2e tests for non-critical workflows. Here's where you could add the test: https://github.com/coder/code-server/blob/main/test/unit/node/cli.test.ts#L53 |
yeah, no problem. I will add a test in login.test.ts and cli.test.ts then, right? |
|
That sounds great. Thank you! |
Signed-off-by: niwla23 <[email protected]>
…r into customize-login-patch
|
I have added the tests |
Signed-off-by: niwla23 <[email protected]>
|
|
||
| it("should return correct welcome text", async () => { | ||
| process.env.PASSWORD = previousEnvPassword | ||
| const welcomeText = "Welcome to your code workspace! öäü🔐" |
There was a problem hiding this comment.
nit: should we use characters without accent marks in case they're rendered differently on someone's machine?
| const welcomeText = "Welcome to your code workspace! öäü🔐" | |
| const welcomeText = "Welcome to your code workspace!" |
There was a problem hiding this comment.
my idea was to test if there are any implications when using unicode, but I don't know if that should be there
There was a problem hiding this comment.
I think that could be tested separately if needed but I personally don't think we need to. thoughts @code-asher ?
There was a problem hiding this comment.
we can also merge now and i can fix later if needed. don't want to hold you back :)
There was a problem hiding this comment.
yeah I don't think it will do any harm, should be ready to merge. I would be glad if you could add the hacktoberfest-accepted label so it counts for https://hacktoberfest.com :)
|
|
||
| it("should return correct app-name", async () => { | ||
| process.env.PASSWORD = previousEnvPassword | ||
| const appName = "testnäme" |
There was a problem hiding this comment.
nit: same as here
| const appName = "testnäme" | |
| const appName = "testname" |
Fixes #5632
adds these config options: