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

Implement IAPIWidget, IButtonWidget and IIconWidget in user status #33940

Merged
merged 3 commits into from
Sep 15, 2022

Conversation

julien-nc
Copy link
Member

@julien-nc julien-nc commented Sep 7, 2022

This is based on #33658

requires:

No buttons for this widget. Use the app svg icon for getIconUrl().

@tobiasKaminsky
Copy link
Member

tobiasKaminsky commented Sep 7, 2022

Web:
image

Android:
image

  • there is a space before "Working…"?
  • icon is not round.
    • Can you also provide this round?
    • Or shall we extend api to say iconRound: true/false
    • or do we wanna do some dark magic and whenever "avatar" is inside icon url, we make it round on client side?

@blizzz blizzz mentioned this pull request Sep 7, 2022
@julien-nc julien-nc force-pushed the dashboard-api-widgets--user_status branch from 3c34d74 to 19b5052 Compare September 7, 2022 20:42
@julien-nc
Copy link
Member Author

@tobiasKaminsky The text includes the status unicode character (the UTF-8 emoji). So when there is no emoji, title starts with a space. This is fixed now.

About the round icon, tough decision to make. Potential solutions, just elaborating what you said:

  1. The dashboard api returns an extra attribute to let the client know if the image should be displayed rounded or not... I would go with that.
  2. Provide it round: It means we have to add a parameter to the avatar server endpoint to optionally get a round image
  3. Dark magic, meh...:grin:

I prefer 1. because it is generic and implemented one single time in each client rather than potentially making the effort of rounding the image in each server widget class.
What do you think?

@tobiasKaminsky
Copy link
Member

Then let us go with 1.
Can I ask you to implement this on main server PR #33658
I think we can make this optional, so by default it is not rounded, and only if rounded the apps has to set it, else API will always send rounded: false.
This is possible nowadays with php, right?

@tobiasKaminsky
Copy link
Member

This lacks IItemOptionWidget so that we have item_icons_round:true ?

@icewind1991 icewind1991 force-pushed the dashboard-api-widgets--user_status branch from 19b5052 to 11391d0 Compare September 13, 2022 11:55
@icewind1991
Copy link
Member

icewind1991 commented Sep 13, 2022

rebased and set item_icons_round

@tobiasKaminsky
Copy link
Member

Tested & worked 👍
Many thanks @eneiluj @icewind1991

@julien-nc
Copy link
Member Author

julien-nc commented Sep 14, 2022

@icewind1991 Thanks for the adjustments!
The target branch of this PR is dashboard-api-widgets so we can merge now.

@skjnldsv skjnldsv mentioned this pull request Sep 15, 2022
Julien Veyssier and others added 3 commits September 15, 2022 17:45
Signed-off-by: Julien Veyssier <eneiluj@posteo.net>
Signed-off-by: Julien Veyssier <eneiluj@posteo.net>
Signed-off-by: Robin Appelman <robin@icewind.nl>
@julien-nc julien-nc force-pushed the dashboard-api-widgets--user_status branch from 11391d0 to 5b25f94 Compare September 15, 2022 15:47
@julien-nc
Copy link
Member Author

Rebased on dashboard-api-widgets.
@tobiasKaminsky One last test and we can merge 🙏

@julien-nc julien-nc merged commit bf3a6c6 into dashboard-api-widgets Sep 15, 2022
@julien-nc julien-nc deleted the dashboard-api-widgets--user_status branch September 15, 2022 16:04
@julien-nc
Copy link
Member Author

Or we can make the final review in the original PR #33658

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants