paper-plane
paper-plane copied to clipboard
Initial document message
I made the support for document message
Todo:
- [X] Remake widget ui
- [x] Add custom css style
- [X] Add action to cancel loading
Make in other requests
- [ ] Add loading indicator
- [ ] Add action to remove the downloaded document
- [ ] Add thumbnails
- [ ] (Idea) add drag and drop to move as file
I'm new to programming using gtk so any suggestions are welcome
Thanks a lot for this PR!
I wrote an initial review about the general widget structure. If you need any help feel free to ask!
Some quick comments about the TODO tasks:
Remake widget ui (using boxes?)
Yeah, I left a review for how this could be managed using GtkBox
es.
Make property to change loading status
I don't think it's really useful having a property for that, you can probably just react to changes in the receiver callback and update the widget.
Add spinner to indicate loading
I'm not sure if you mean an actual progress animation or just a GtkSpinner
. For the former we'd need a custom widget to achieve the same animation as seen in other Telegram clients (radial progress bar). For the latter, instead, I think it's not a great option because it can't show the progress, so we could just show the cancel button to indicate that it's downloading.
Add action to remove the downloaded document
I don't think this is really useful, I don't see other clients doing something like this.
Add thumbnails (low priority)
I think you can skip this for this PR so that we can concentrate about the basic widget first.
I updated ui but now i can't open some files
upd: now for some reason i can
Maybe add different colors for different file types like Telegram X, but i don't found this in standart clients
The problem with this is that it's really difficult to recolor the app (we'll support the official themes as seen in the official clients) and still assure a good contrast for every file type color.
About CSS style I suggest something like this:
messagedocument > box > button {
background: @accent_color;
color: @window_bg_color;
}
messagedocument.outgoing > box > button {
background: @accent_fg_color;
color: @accent_bg_color;
}
This will result in this result, which assures a good contrast (ignore the low contrast sender color, that's a different bug :p):
Now i can cancel downlading
Other changes:
- I've changed some logic in code for ease of handling different statuses
- Now the status_button doesn't accumulate handlers (I thought that Button::connect_clicked automatically deletes the old action)
- Now loading status hides size of file (i think without automatic downloading it's useless)
- Style is updated
Playing with thumbnails, but i don't know good way to fit image into square
here's the bad way:
let pixbuf =
gdk::gdk_pixbuf::Pixbuf::from_file(&thumbnail.file.local.path).unwrap();
let new_pixbuf = gdk::gdk_pixbuf::Pixbuf::new(
gdk::gdk_pixbuf::Colorspace::Rgb,
false,
pixbuf.bits_per_sample(),
320,
320,
)
.unwrap();
let scale = thumbnail.width as f64 / thumbnail.height as f64;
let offset_x = -(thumbnail.width as f64 - thumbnail.height as f64);
pixbuf.composite(
&new_pixbuf,
0,
0,
320,
320,
offset_x,
0.0,
scale,
scale,
gdk::gdk_pixbuf::InterpType::Nearest,
255,
);
let pixbuf = new_pixbuf;
let picture = gtk::Picture::for_pixbuf(&pixbuf);
Playing with thumbnails, but i don't know good way to fit image into square
We probably want to wait for GTK 4.8 where I've added a property to GtkPicture
to control just that. For what I've heard a new version of gtk4-rs is coming soon with the new APIs, so we'll switch to 4.8 when that release will happen.
Almost there. I see some remaining issues regarding the commits:
Commit Message
The first word after the tag should be capitalized, and you should use imperative style
message-document: added clickable area -> message-document: Add clickable area
Split of the Commits
I see some issues how you split up the commits. For example, you fix style issues in the second commit that you introduced in the first commit. That produces a lot of noise because the information that you fixed things within the PR isn't relevant after it has been merged. Besides, it distracts the reviewer.
A better split would be to introduce the necessary types (document) in the first commit and wire things up in the second commit (show document messages in the chat). But I think, in this case, it's also fine to squash everything into one commit.
Very Good. You have my approval if you add an imperative verb to the summary line of the commit message. For example message-document: Implement initial version
or message-document: Implement basic functionality
For commit guidelines you can refer to https://wiki.gnome.org/Git/CommitMessages
Last thing, do you mind squashing everything into a single commit?
Thanks!