Skip to content

bug: Logo URL in Appearance dashboard does not explain that it also updates the logo on the Sign In page #11345

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

Closed
sharkymark opened this issue Dec 28, 2023 · 9 comments · Fixed by #11500
Assignees
Labels
s3 Bugs that confuse, annoy, or are purely cosmetic site Area: frontend dashboard

Comments

@sharkymark
Copy link
Contributor

The help text for the Logo URL in Appearance dashboard does not correctly explain that it also updates the logo at the top of the Sign In page. It does update the logo at the top left of the main dashboard too.

image image

This a second issue, but changing the Application Name in the Appearance dashboard does not change anything on the Sign In page.

@sharkymark sharkymark added the bug label Dec 28, 2023
@matifali
Copy link
Member

What do you suggest as the solution? Improve the wording or suport separate logos for both places?

@sharkymark
Copy link
Contributor Author

The helper text needs to mention this name also updates the sign in page

The second issue is a bug I think - if you want to test. Changing that text did nothing to my sign in page.

@matifali matifali added site Area: frontend dashboard s3 Bugs that confuse, annoy, or are purely cosmetic labels Dec 30, 2023
@f0ssel
Copy link
Contributor

f0ssel commented Jan 2, 2024

Went ahead and made a fix for the wording in #11377. I can look at the Application Name issue if product believes this is a bug but unfamiliar with the decisions and changes when they were made so I don't want to jump to conclusions that something needs fixing there.

@f0ssel f0ssel self-assigned this Jan 2, 2024
@f0ssel
Copy link
Contributor

f0ssel commented Jan 4, 2024

So I looked into this further and the application name is only used on the frontend to provide an alt image text to the sign in page logo. https://github.com/coder/coder/blob/f0ssel/templatepush-2/site/src/pages/LoginPage/LoginPageView.tsx#L35

This means the Application Name would only be displayed if the image failed to load or if the user is using a screenreader.

This code looks pretty intentional, so I'm going to assume it isn't a bug but that the wording under this field is also slightly confusing. So I propose:

  1. Fix the wording to make it clear it's only used for fallback if the image cannot load.
  2. Make some frontend design changes to include the application name on the sign in page somewhere that isn't as hidden as image alt text.

I probably am not the best at the design change part but can implement whatever the frontend team would like to do.

CC @BrunoQuaresma @matifali

@matifali
Copy link
Member

matifali commented Jan 4, 2024

@f0ssel I think this was an overlook. We do want to Display both the logo and name.
cc: @bpmct

@BrunoQuaresma
Copy link
Collaborator

Just to take into consideration: A few customers shared that they found the old heading "Sign in to Company Name" quite overwhelming and I kinda agree with that. That was the reason why I simplified it in the last sign-in page refactoring.

We should improve the settings page where the user can upload the logo and let the user decide what they want to do:

  • Display logo on the sign-in page
  • Display logo on dashboard top left
  • Use the company name on the sign-in page

@f0ssel
Copy link
Contributor

f0ssel commented Jan 4, 2024

@BrunoQuaresma Do you think those same companies would fine the default "Sign in to Coder" overwhelming? I can see why someone setting it to "CartoonNetwork" might find "Sign in to CartoonNetwork" confusing, but I feel like the default should be sane enough to not need another toggle switch IMO.

@BrunoQuaresma
Copy link
Collaborator

@f0ssel yeap, I'm going to share it with you privately.

@f0ssel
Copy link
Contributor

f0ssel commented Jan 8, 2024

I went ahead and made a PR #11500 to display just the application name here instead. @sharkymark @matifali @BrunoQuaresma Please let me know if you think this makes sense and would please our current clients. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
s3 Bugs that confuse, annoy, or are purely cosmetic site Area: frontend dashboard
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants