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

Application definition "icons" property #257

Closed
pgn-vole opened this issue Oct 12, 2020 · 5 comments · Fixed by #319
Closed

Application definition "icons" property #257

pgn-vole opened this issue Oct 12, 2020 · 5 comments · Fixed by #319
Milestone

Comments

@pgn-vole
Copy link

pgn-vole commented Oct 12, 2020

Question Area

[X] App Directory
[ ] API
[ ] Context Data
[ ] Intents
[ ] Use Cases
[ ] Other

Question

Could you please clarify how does an application launcher should interpret the icons property of applications within the app directory?

It is specified for this property to have the below shape:

{
 // other app properties
 icons : [
   {  
     icon: "https://foo.com/icon.png"
   }
 ]
}

Since icons is a list, which item of that list should the launcher use and as an application developer how do I know which item of that list the launcher is going to pick?

I had a look at 2 open source implementations [1] and the logic appears flimsy to me considering that they do arbitrarily pick the first item of that list.

I would think in the first place that multiple icons would be useful so that different sizes of the same icon or even different icons could be exposed to launchers. However to enable this capability some metadata properties would have to be added to the "icon" model. At the moment it does support a single property "icon" which is meant to be just the url.

1: https://github.com/ChartIQ/finsemble-fdc3/blob/master/src/components/intentResolver/src/app.tsx#L204
https://github.com/finos/fdc3-desktop-agent/blob/master/src/content.ts#L338

@nkolba
Copy link
Contributor

nkolba commented Oct 17, 2020

Totally agree. Would make sense to base on how this is handled in web app manifest.

@rikoe
Copy link
Contributor

rikoe commented Oct 25, 2020

If someone is willing to do a PR, I propose we add this to the scope for 1.2?

@pgn-vole
Copy link
Author

I'm happy to take ownership on that and raise a relevant PR if that's fine with you?

@rikoe rikoe added this to the 2.0 milestone Jan 28, 2021
@rikoe
Copy link
Contributor

rikoe commented Jan 28, 2021

@pgn-vole are you still planning to submit a PR for this one? Would be great to get this into 2.0 as part of other app directory work.

@pgn-vole
Copy link
Author

pgn-vole commented Feb 1, 2021

@rikoe, I'm on it. Will raise a PR sometime this week.

@pgn-vole pgn-vole mentioned this issue Feb 8, 2021
@rikoe rikoe modified the milestones: 2.0-candidates, 2.0 Apr 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants