From c56c78f86ec37eb25a15ea7b65851dfcd0b5fab3 Mon Sep 17 00:00:00 2001 From: Marcus Behrendt Date: Tue, 3 Oct 2023 15:18:34 +0200 Subject: [PATCH] refactor(chat-history-model): Use sections as day dividers Using sections simplifies our code a lot. It still needs changes applied to the reversed list patch, otherwise sections are shifted forward by one position. --- src/model/chat_history_item.rs | 92 --------- src/model/chat_history_model.rs | 219 +++++++-------------- src/model/mod.rs | 3 - src/ui/session/content/chat_history.rs | 36 ++++ src/ui/session/content/chat_history.ui | 6 + src/ui/session/content/chat_history_row.rs | 66 +++---- 6 files changed, 133 insertions(+), 289 deletions(-) delete mode 100644 src/model/chat_history_item.rs diff --git a/src/model/chat_history_item.rs b/src/model/chat_history_item.rs deleted file mode 100644 index f8c1dff11..000000000 --- a/src/model/chat_history_item.rs +++ /dev/null @@ -1,92 +0,0 @@ -use std::cell::OnceCell; - -use glib::prelude::*; -use glib::subclass::prelude::*; -use gtk::glib; -use once_cell::sync::Lazy; - -use crate::model; - -#[derive(Clone, Debug, glib::Boxed)] -#[boxed_type(name = "ChatHistoryItemType")] -pub(crate) enum ChatHistoryItemType { - Message(model::Message), - DayDivider(glib::DateTime), -} - -mod imp { - use super::*; - - #[derive(Debug, Default)] - pub(crate) struct ChatHistoryItem { - pub(super) type_: OnceCell, - } - - #[glib::object_subclass] - impl ObjectSubclass for ChatHistoryItem { - const NAME: &'static str = "ChatHistoryItem"; - type Type = super::ChatHistoryItem; - } - - impl ObjectImpl for ChatHistoryItem { - fn properties() -> &'static [glib::ParamSpec] { - static PROPERTIES: Lazy> = Lazy::new(|| { - vec![glib::ParamSpecBoxed::builder::("type") - .write_only() - .construct_only() - .build()] - }); - PROPERTIES.as_ref() - } - - fn set_property(&self, _id: usize, value: &glib::Value, pspec: &glib::ParamSpec) { - match pspec.name() { - "type" => { - let type_ = value.get::().unwrap(); - self.type_.set(type_).unwrap(); - } - _ => unimplemented!(), - } - } - } -} - -glib::wrapper! { - pub(crate) struct ChatHistoryItem(ObjectSubclass); -} - -impl ChatHistoryItem { - pub(crate) fn for_message(message: model::Message) -> Self { - let type_ = ChatHistoryItemType::Message(message); - glib::Object::builder().property("type", type_).build() - } - - pub(crate) fn for_day_divider(day: glib::DateTime) -> Self { - let type_ = ChatHistoryItemType::DayDivider(day); - glib::Object::builder().property("type", type_).build() - } - - pub(crate) fn type_(&self) -> &ChatHistoryItemType { - self.imp().type_.get().unwrap() - } - - pub(crate) fn message(&self) -> Option<&model::Message> { - if let ChatHistoryItemType::Message(message) = self.type_() { - Some(message) - } else { - None - } - } - - pub(crate) fn message_timestamp(&self) -> Option { - if let ChatHistoryItemType::Message(message) = self.type_() { - Some( - glib::DateTime::from_unix_utc(message.date().into()) - .and_then(|t| t.to_local()) - .unwrap(), - ) - } else { - None - } - } -} diff --git a/src/model/chat_history_model.rs b/src/model/chat_history_model.rs index 98c79f4df..451ec8099 100644 --- a/src/model/chat_history_model.rs +++ b/src/model/chat_history_model.rs @@ -1,13 +1,12 @@ use std::cell::Cell; use std::cell::RefCell; -use std::cmp::Ordering; use std::collections::VecDeque; use gio::prelude::*; -use gio::subclass::prelude::*; use glib::clone; use gtk::gio; use gtk::glib; +use gtk::subclass::prelude::*; use once_cell::sync::Lazy; use thiserror::Error; @@ -28,14 +27,14 @@ mod imp { pub(crate) struct ChatHistoryModel { pub(super) chat: glib::WeakRef, pub(super) is_loading: Cell, - pub(super) list: RefCell>, + pub(super) list: RefCell>, } #[glib::object_subclass] impl ObjectSubclass for ChatHistoryModel { const NAME: &'static str = "ChatHistoryModel"; type Type = super::ChatHistoryModel; - type Interfaces = (gio::ListModel,); + type Interfaces = (gio::ListModel, gtk::SectionModel); } impl ObjectImpl for ChatHistoryModel { @@ -60,7 +59,7 @@ mod imp { impl ListModelImpl for ChatHistoryModel { fn item_type(&self) -> glib::Type { - model::ChatHistoryItem::static_type() + model::Message::static_type() } fn n_items(&self) -> u32 { @@ -75,6 +74,26 @@ mod imp { .cloned() } } + + impl SectionModelImpl for ChatHistoryModel { + fn section(&self, position: u32) -> (u32, u32) { + let list = &*self.list.borrow(); + let message = list.get(position as usize).unwrap(); + + let ymd = glib::DateTime::from_unix_local(message.date() as i64) + .unwrap() + .ymd(); + + ( + if position == 0 { + 0 + } else { + find_section_border(list, ymd, position as i32 - 1, true) + }, + find_section_border(list, ymd, position as i32 + 1, false), + ) + } + } } glib::wrapper! { @@ -108,14 +127,7 @@ impl ChatHistoryModel { return Err(ChatHistoryError::AlreadyLoading); } - let oldest_message_id = imp - .list - .borrow() - .iter() - .rev() - .find_map(|item| item.message()) - .map(|m| m.id()) - .unwrap_or_default(); + let oldest_message_id = imp.list.borrow().back().map(|m| m.id()).unwrap_or_default(); imp.is_loading.set(true); @@ -133,125 +145,21 @@ impl ChatHistoryModel { Ok(true) } - fn items_changed(&self, position: u32, removed: u32, added: u32) { - let imp = self.imp(); - - // Insert day dividers where needed - let added = { - let position = position as usize; - let added = added as usize; - - let mut list = imp.list.borrow_mut(); - let mut previous_timestamp = if position + 1 < list.len() { - list.get(position + 1) - .and_then(|item| item.message_timestamp()) - } else { - None - }; - let mut dividers: Vec<(usize, model::ChatHistoryItem)> = vec![]; - - for (index, current) in list.range(position..position + added).enumerate().rev() { - if let Some(current_timestamp) = current.message_timestamp() { - if Some(current_timestamp.ymd()) != previous_timestamp.as_ref().map(|t| t.ymd()) - { - let divider_pos = position + index + 1; - dividers.push(( - divider_pos, - model::ChatHistoryItem::for_day_divider(current_timestamp.clone()), - )); - previous_timestamp = Some(current_timestamp); - } - } - } - - let dividers_len = dividers.len(); - for (position, item) in dividers { - list.insert(position, item); - } - - (added + dividers_len) as u32 - }; - - // Check and remove no more needed day divider after removing messages - let removed = { - let mut removed = removed as usize; - - if removed > 0 { - let mut list = imp.list.borrow_mut(); - let position = position as usize; - let item_before_removed = list.get(position); - - if let Some(model::ChatHistoryItemType::DayDivider(_)) = - item_before_removed.map(|i| i.type_()) - { - let item_after_removed = if position > 0 { - list.get(position - 1) - } else { - None - }; - - match item_after_removed.map(|item| item.type_()) { - None | Some(model::ChatHistoryItemType::DayDivider(_)) => { - list.remove(position + removed); - - removed += 1; - } - _ => {} - } - } - } - - removed as u32 - }; - - // Check and remove no more needed day divider after adding messages - let (position, removed) = { - let mut removed = removed; - let mut position = position as usize; - - if added > 0 && position > 0 { - let mut list = imp.list.borrow_mut(); - let last_added_timestamp = list.get(position).unwrap().message_timestamp().unwrap(); - let next_item = list.get(position - 1); - - if let Some(model::ChatHistoryItemType::DayDivider(date)) = - next_item.map(|item| item.type_()) - { - if date.ymd() == last_added_timestamp.ymd() { - list.remove(position - 1); - - removed += 1; - position -= 1; - } - } - } - - (position as u32, removed) - }; - - self.upcast_ref::() - .items_changed(position, removed, added); - } - fn push_front(&self, message: model::Message) { - self.imp() - .list - .borrow_mut() - .push_front(model::ChatHistoryItem::for_message(message)); + self.imp().list.borrow_mut().push_front(message); self.items_changed(0, 0, 1); } fn append(&self, messages: Vec) { let imp = self.imp(); + let added = messages.len(); imp.list.borrow_mut().reserve(added); for message in messages { - imp.list - .borrow_mut() - .push_back(model::ChatHistoryItem::for_message(message)); + imp.list.borrow_mut().push_back(message); } let index = imp.list.borrow().len() - added; @@ -259,44 +167,51 @@ impl ChatHistoryModel { } fn remove(&self, message: model::Message) { - let imp = self.imp(); - - // Put this in a block, so that we only need to borrow the list once and the runtime - // borrow checker does not panic in Self::items_changed when it borrows the list again. - let index = { - let mut list = imp.list.borrow_mut(); - - // The elements in this list are ordered. While the day dividers are ordered - // only by their date time, the messages are additionally sorted by their id. We - // can exploit this by applying a binary search. - let index = list - .binary_search_by(|m| match m.type_() { - model::ChatHistoryItemType::Message(other_message) => { - message.id().cmp(&other_message.id()) - } - model::ChatHistoryItemType::DayDivider(date_time) => { - let ordering = glib::DateTime::from_unix_utc(message.date() as i64) - .unwrap() - .cmp(date_time); - if let Ordering::Equal = ordering { - // We found the day divider of the message. Therefore, the message - // must be among the following elements. - Ordering::Greater - } else { - ordering - } - } - }) - .unwrap(); + let mut list = self.imp().list.borrow_mut(); + if let Ok(index) = list.binary_search_by(|m| message.id().cmp(&m.id())) { list.remove(index); - index as u32 - }; - self.items_changed(index, 1, 0); + drop(list); + self.items_changed(index as u32, 1, 0); + } } pub(crate) fn chat(&self) -> model::Chat { self.imp().chat.upgrade().unwrap() } } + +fn find_section_border( + messages: &VecDeque, + ymd: (i32, i32, i32), + position: i32, + backwards: bool, +) -> u32 { + if position < 0 { + return position as u32; + } + + match messages.get(position as usize) { + None => position as u32, + Some(message) => { + let date_time = glib::DateTime::from_unix_local(message.date() as i64).unwrap(); + if date_time.ymd() == ymd { + find_section_border( + messages, + ymd, + if backwards { + position - 1 + } else { + position + 1 + }, + backwards, + ) + } else if backwards { + position as u32 + 1 + } else { + position as u32 + } + } + } +} diff --git a/src/model/mod.rs b/src/model/mod.rs index bf22afe2a..d6b23f749 100644 --- a/src/model/mod.rs +++ b/src/model/mod.rs @@ -3,7 +3,6 @@ mod basic_group; mod chat; mod chat_action; mod chat_action_list; -mod chat_history_item; mod chat_history_model; mod chat_list; mod chat_list_item; @@ -52,8 +51,6 @@ pub(crate) use self::chat::Chat; pub(crate) use self::chat::ChatType; pub(crate) use self::chat_action::ChatAction; pub(crate) use self::chat_action_list::ChatActionList; -pub(crate) use self::chat_history_item::ChatHistoryItem; -pub(crate) use self::chat_history_item::ChatHistoryItemType; pub(crate) use self::chat_history_model::ChatHistoryError; pub(crate) use self::chat_history_model::ChatHistoryModel; pub(crate) use self::chat_list::ChatList; diff --git a/src/ui/session/content/chat_history.rs b/src/ui/session/content/chat_history.rs index 28a706c6f..7fb6229bd 100644 --- a/src/ui/session/content/chat_history.rs +++ b/src/ui/session/content/chat_history.rs @@ -50,6 +50,7 @@ mod imp { fn class_init(klass: &mut Self::Class) { klass.bind_template(); + klass.bind_template_callbacks(); klass.install_action("chat-history.view-info", None, move |widget, _, _| { widget.open_info_dialog(); @@ -170,6 +171,41 @@ mod imp { } impl BinImpl for ChatHistory {} + + #[gtk::template_callbacks] + impl ChatHistory { + #[template_callback] + fn on_signal_list_item_factory_bind_header(&self, list_item: &glib::Object) { + let list_header = list_item.downcast_ref::().unwrap(); + + if let Some(item) = list_header.item() { + let message = item.downcast::().unwrap(); + + let date_time = glib::DateTime::from_unix_local(message.date() as i64).unwrap(); + + let fmt = if date_time.year() == glib::DateTime::now_local().unwrap().year() { + // Translators: This is a date format in the day divider without the year + gettext("%B %e") + } else { + // Translators: This is a date format in the day divider with the year + gettext("%B %e, %Y") + }; + + let row = ui::EventRow::new(); + row.set_label(&date_time.format(&fmt).unwrap()); + + list_header.set_child(Some(&row)); + } + } + + #[template_callback] + fn on_signal_list_item_factory_unbind_header(&self, list_item: &glib::Object) { + list_item + .downcast_ref::() + .unwrap() + .set_child(gtk::Widget::NONE); + } + } } glib::wrapper! { diff --git a/src/ui/session/content/chat_history.ui b/src/ui/session/content/chat_history.ui index 34d78ac31..efdbe3504 100644 --- a/src/ui/session/content/chat_history.ui +++ b/src/ui/session/content/chat_history.ui @@ -94,6 +94,12 @@ + + + + + + >, } @@ -85,47 +84,30 @@ impl ChatHistoryRow { } if let Some(ref item) = item { - if let Some(item) = item.downcast_ref::() { - match item.type_() { - model::ChatHistoryItemType::Message(message) => { - use tdlib::enums::MessageContent::*; - - match message.content().0 { - MessageExpiredPhoto - | MessageExpiredVideo - | MessageCall(_) - | MessageBasicGroupChatCreate(_) - | MessageSupergroupChatCreate(_) - | MessageChatChangeTitle(_) - | MessageChatChangePhoto(_) // TODO: Show photo thumbnail - | MessageChatDeletePhoto - | MessageChatAddMembers(_) - | MessageChatJoinByLink - | MessageChatJoinByRequest - | MessageChatDeleteMember(_) - | MessagePinMessage(_) - | MessageScreenshotTaken - | MessageGameScore(_) - | MessageContactRegistered => { - self.get_or_create_event_row() - .set_label(&strings::message_content(message)); - } - _ => self.update_or_create_message_row(message.to_owned().upcast()), - } - } - model::ChatHistoryItemType::DayDivider(date) => { - let fmt = if date.year() == glib::DateTime::now_local().unwrap().year() { - // Translators: This is a date format in the day divider without the year - gettext("%B %e") - } else { - // Translators: This is a date format in the day divider with the year - gettext("%B %e, %Y") - }; - let date = date.format(&fmt).unwrap().to_string(); - - let child = self.get_or_create_event_row(); - child.set_label(&date); + if let Some(message) = item.downcast_ref::() { + use tdlib::enums::MessageContent::*; + + match message.content().0 { + MessageExpiredPhoto + | MessageExpiredVideo + | MessageCall(_) + | MessageBasicGroupChatCreate(_) + | MessageSupergroupChatCreate(_) + | MessageChatChangeTitle(_) + | MessageChatChangePhoto(_) // TODO: Show photo thumbnail + | MessageChatDeletePhoto + | MessageChatAddMembers(_) + | MessageChatJoinByLink + | MessageChatJoinByRequest + | MessageChatDeleteMember(_) + | MessagePinMessage(_) + | MessageScreenshotTaken + | MessageGameScore(_) + | MessageContactRegistered => { + self.get_or_create_event_row() + .set_label(&strings::message_content(message)); } + _ => self.update_or_create_message_row(message.to_owned().upcast()), } } else if let Some(sponsored_message) = item.downcast_ref::() { let content = &sponsored_message.content().0;