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

Add video chat instances icons #3702

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

Conversation

farooqkz
Copy link
Collaborator

@farooqkz farooqkz commented Feb 22, 2024

This PR aims to do just these two:

  • Fetch video chat instance's favicon and save them in a json file in src
  • Show those icons in the frontend
  • remove the initial render delay(about 1s on my machine).

@Simon-Laux
Copy link
Member

Simon-Laux commented Feb 23, 2024

Fetch video chat instance's favicon and save them in a json file in src

I think it makes not much sense to do it on every build because those won't change very often.
This will make development slower by making "npm run build" slower, also it will fail on flatpak as you have no internet access there.

you can also save the icons of the default instances as icons to a subfolder of images/

Then the format of the video chat provider objects could be:

interface VideoChatInstance {
 // id for distinguishing between the instances
 id: string,
 // the url of the video chat provider that is given to core
 url: string,
 // path of the icon, either relative url for default providers or base64 for custom providers that user added that were saved in a core ui config
 icon: string, 
}
const VIDEO_CHAT_INSTANCES: Record<string, VideoChatInstance > = {
'systemli': { id: 'systemli', url: 'https://meet.systemli.org/$ROOM', icon: "../images/videochat_providers/systemli.ico" },
'autistici': { id: 'autistici', url: 'https://vc.autistici.org/$ROOM', icon: "../images/videochat_providers/autistici" },
}

and maybe also a display name property that we wanna show to the user

@farooqkz farooqkz marked this pull request as ready for review March 10, 2024 13:56
@farooqkz
Copy link
Collaborator Author

@Simon-Laux @adzialocha I need to investigate why the initial render takes so much time but hints are welcome.

@Simon-Laux
Copy link
Member

needs to be rebased

let data = {}
for (const [id, props] of Object.entries(VIDEO_CHAT_INSTANCES)) {
for (const i = 1; i <= 3; i++) { // max 3 retries
const response = await fetch(props.url.replace('$ROOM', '/images/favicon.ico'))
Copy link
Member

Choose a reason for hiding this comment

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

might work, but parsing url and just taking hostname + /favicon.ico might be better. but ok for now as long as the default instances are jitsi

but do not write code to parse urls yourself, just use https://nodejs.org/docs/latest-v18.x/api/url.html

@Simon-Laux
Copy link
Member

might make sense to show the icons pixelated, as blurry does not look good. (css image-rendering: pixelated;)
We can also try to get the icons from different places in the hope to get a higher resolution one (fetching the html and parsing the xml to get the head elements content:

  • <link rel="apple-touch-icon" href="images/apple-touch-icon.png">
  • icons referenced in a webmanifest: <link rel="manifest" href="manifest.json" />
  • <link rel="icon" type="image/png" href="images/favicon.ico?v=1">

margin: '4px',
borderRadius: iconRound ? '4px' : undefined,
backgroundColor: iconWithBackground
? 'var(--textPrimary)'
Copy link
Member

Choose a reason for hiding this comment

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

should be hardcoded to white, most icons are deigned for light mode, this would ensure that the icon works also in darkmode

@Simon-Laux
Copy link
Member

But I'm not sure it's wise to prioritise this right now as there is still quite some stuff left todo to make it really useful, also I think the code structure can be much simplified.

@Simon-Laux Simon-Laux marked this pull request as draft April 8, 2024 18:01
@farooqkz farooqkz marked this pull request as ready for review May 13, 2024 14:05
@farooqkz
Copy link
Collaborator Author

This PR is ready for review and just adds the icons to make it look prettier a bit. The option to add feature to let users add their own instances will be added through another PR.

@farooqkz
Copy link
Collaborator Author

I think I did a wrong rebase :D

@farooqkz farooqkz requested a review from Simon-Laux May 13, 2024 14:09
@farooqkz farooqkz force-pushed the far-videochat-instances-icon branch from 26adf8f to 763428b Compare May 14, 2024 14:24
@farooqkz farooqkz force-pushed the far-videochat-instances-icon branch from 763428b to cba7f87 Compare May 15, 2024 14:12
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.

None yet

2 participants