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

feat: Set summary thumbnail path for WebXDCs to "webxdc-icon://last-msg-id" (#5782) #5789

Merged
merged 1 commit into from
Jul 22, 2024

Conversation

iequidoo
Copy link
Collaborator

This is a hint for apps that a WebXDC icon should be shown in the summary, e.g. in the chatlist. Otherwise it's not clear when it should be shown, e.g. it shouldn't be shown in a reaction summary.

#5782 should be closed after the corresponding DC Desktop fix.

src/summary.rs Outdated
@@ -129,6 +129,8 @@ impl Summary {
{
msg.get_file(context)
.and_then(|path| path.to_str().map(|p| p.to_owned()))
} else if msg.viewtype == Viewtype::Webxdc {
Some("msg-icon://last-msg-id".to_string())
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The suggestion was "webxdc-icon:last-msg-id", but i think:

  • This should be path-like, so let's use "msg-icon://" as a prefix
  • Better not to add "webxdc", this way the string can be reused for other viewtypes if we want. It's just a hint, how the icon should be taken exactly is up to the app

Copy link
Member

Choose a reason for hiding this comment

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

actually it should contain webxdc IMO, because the ui in desktop just replaces it with the url for the custom uri scheme that serves webxdc icons. I don't see much point in having an api that requires an extra if statement in ui code.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I changed it to "webxdc-icon://last-msg-id"

@iequidoo iequidoo marked this pull request as ready for review July 21, 2024 19:52
…sg-id" (#5782)

This is a hint for apps that a WebXDC icon should be shown in the summary, e.g. in the
chatlist. Otherwise it's not clear when it should be shown, e.g. it shouldn't be shown in a reaction
summary.
@iequidoo iequidoo force-pushed the iequidoo/webxdc-summary-icon branch from f41df0c to f90171c Compare July 21, 2024 21:13
@iequidoo iequidoo changed the title feat: Set summary thumbnail path for WebXDCs to "msg-icon://last-msg-id" (#5782) feat: Set summary thumbnail path for WebXDCs to "webxdc-icon://last-msg-id" (#5782) Jul 21, 2024
Copy link
Member

@Simon-Laux Simon-Laux left a comment

Choose a reason for hiding this comment

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

code looks fine, didn't test

@iequidoo iequidoo merged commit bd651d9 into main Jul 22, 2024
35 of 37 checks passed
@iequidoo iequidoo deleted the iequidoo/webxdc-summary-icon branch July 22, 2024 21:25
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.

2 participants