-
Notifications
You must be signed in to change notification settings - Fork 213
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
Moving static data out of Element client file #273
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,36 @@ | ||
import { Platform } from "../types.js"; | ||
|
||
export class Client { | ||
constructor(data) { | ||
this.data = data; | ||
} | ||
|
||
get id() { return this.data.id; } | ||
|
||
get platforms() { return this.data.platforms; } | ||
|
||
get icon() { return "images/client-icons/"+this.data.icon; } | ||
thibaultamartin marked this conversation as resolved.
Show resolved
Hide resolved
|
||
get appleAssociatedAppId() { return this.data.appleAssociatedAppId; } | ||
get name() {return this.data.name; } | ||
get description() { return this.data.description; } | ||
get homepage() { return this.data.homepage; } | ||
get author() { return this.data.author; } | ||
getMaturity(platform) { return this.data.maturity; } | ||
|
||
getLinkInstructions(platform, link) {} | ||
getCopyString(platform, link) {} | ||
getInstallLinks(platform) { | ||
var links = []; | ||
if (platform === Platform.iOS && this.platforms().includes(Platform.iOS) && this.data.applestorelink) { | ||
links.push(this.data.applestorelink); | ||
} else if (platform === Platform.Android && this.platforms.includes(Platform.Android)) { | ||
if (this.data.playstorelink) { links.push(this.data.playstorelink); } | ||
if (this.data.fdroidlink) { links.push(this.data.fdroidlink); } | ||
} | ||
else if (this.data.defaultInstallLink) { links.push(this.data.defaultInstallLink); } | ||
|
||
return links; | ||
} | ||
|
||
canInterceptMatrixToLinks(platform) { return false; } | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
import {Maturity, Platform, FDroidLink, AppleStoreLink, PlayStoreLink, WebsiteLink, FlathubLink} from "../types.js"; | ||
|
||
export const data = { | ||
"id": "element.io", | ||
"platforms": [Platform.Android, Platform.iOS, Platform.Windows, Platform.macOS, Platform.Linux, Platform.DesktopWeb], | ||
"icon": "element.svg", | ||
"appleAssociatedAppId": "7J4U792NQT.im.vector.app", | ||
thibaultamartin marked this conversation as resolved.
Show resolved
Hide resolved
|
||
"name": "Element", | ||
"description": "Fully-featured Matrix client, used by millions.", | ||
"homepage": "https://element.io", | ||
"author": "Element", | ||
"maturity": Maturity.Stable, | ||
"applestorelink": new AppleStoreLink('vector', 'id1083446067'), | ||
"playstorelink": new PlayStoreLink('im.vector.app'), | ||
"fdroidlink": new FDroidLink('im.vector.app'), | ||
"flathublink": new FlathubLink('im.riot.Riot'), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why not make these links something like this? You can wrap these strings into their *Link objects in Client.getInstallLinks, that way it's closer to json and people don't have to deal with knowing about these classes:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think at that point "installLinks" becomes misleading, because There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would like as much of the content as possible to be generated by squiddy (to make sure it will be kept up to date). The file format for squiddy is not completely dry yet, but for clients it would look like [[clients]]
title = "Element Web"
description = "Element is a glossy web client with an emphasis on performance and usability"
maturity = "Stable"
(more fields…)
appstore_details.org = "vector"
appstore_details.app_id = "id1083446067"
playstore_app_id = "im.vector.app"
fdroid_app_id = "im.vector.app"
flathub_app_id = "im.vector.app"
otherinstall_link = ["https://element.io/get-started"]
full_description = """
Longer multi-line
description of the client
that can [include links](#) and `markdown in general`
"""
[[clients.authors]]
name = "Element"
[[clients.authors]]
name = "Kat"
matrix_id = "@kittykat:matrix.org"
[[clients.authors]]
name = "Danielle Kirkwood"
matrix_id = "@daniellekirkwood:one.ems.host" Under those circumstances, it's not a huge problem to use the classes. Is it acceptable for you? |
||
"defaultInstallLink": new WebsiteLink("https://element.io/get-started"), | ||
}; |
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.
Is it okay to snake case the project name as an id?
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.
not sure what you mean here. this line seems fine to me.
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.
@bwindels right now
id
is a field that only exist for matrix.toAt the moment for Element, the
id
iselement.io
.Would it be ok to use
element
instead? I would derive theid
from the project name (since two projects can't have the same name anyway)