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

Update webmanifest to include screenshots #4806

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jstjnsn
Copy link

@jstjnsn jstjnsn commented Dec 9, 2023

I was not able to reproduce the issue mentioned locally but these changes should fix the problems mentioned. Please let me know if you approve of these changes.

client/thelounge.webmanifest Outdated Show resolved Hide resolved
@xPaw
Copy link
Member

xPaw commented Dec 10, 2023

See https://web.dev/patterns/web-apps/richer-install-ui for more information. In chrome devtools, you can see warnings by going to Application > Manifest tab.

As for screenshots, I've found that on desktop they scale very poorly, so it needs to be zoomed in a lot. As well as private information such as IPs from your join message, and the user list should not include random users (for their sake). Perhaps a few messages should be sent to show link previews etc.

Perhaps another screenshot with the server list open could be included for mobile.

@xPaw xPaw changed the title Should fix issue #4800 Update webmanifest to include screenshots Dec 10, 2023
@brunnre8
Copy link
Member

brunnre8 commented Dec 13, 2023

Considering that the screenshots are meant to showcase the app a bit, showing the server window is probably not quite the right thing to do.
Let's synthesize some interesting content in some test / dummy chan instead.

Link to https://thelounge.chat, hold a convo with some dummy users or whatever.
Does this make sense?

Essentially the goal is to be as realistic as possible, showcasing the features, without putting anyone's privacy at risk that didn't give their consent.

Feel free to use #thelounge-test, that's just "wendy" and I (bookworm), both are non secret so feel free to bug wendy with commands / expose our IPs etc.

both are joined from hosts I control.

@jstjnsn
Copy link
Author

jstjnsn commented Dec 20, 2023

@brunnre8 I'm not sure I understand, I tried to join thelounge-test channel and chat, but I am not getting any responses. Is it good enough if I just add the following?

  • screenshot of the homepage login window
  • screenshot of the thelounge-test channel with a message I sent
    anything else?

@brunnre8
Copy link
Member

brunnre8 commented Dec 26, 2023

Do you understand why the linter yells at us?
As in, can you follow the reasoning as to why we need the pictures?

I don't understand why you'd want to show a login screen? It's not interesting.

You want to have pictures which are appealing, showcasing what the app can do.
Having a channel with one message doesn't do the trick.

https://thelounge.chat/img/thelounge-screenshot.png

^ this is an example from our webpage, see it showcases some fun things you can't do in other clients... audio, link preview etc.
Multiple users... Those things.

THAT is what this issue is about, not just putting some random images into our repo to shut the linter up.

@brunnre8 brunnre8 added Type: Feature Tickets that describe a desired feature or PRs that add them to the project. Status: changes-requested Review happened but some changes need to be made labels Jan 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: changes-requested Review happened but some changes need to be made Type: Feature Tickets that describe a desired feature or PRs that add them to the project.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants