fix: raw md page has no titlebar on macOS#238
Merged
lollipopkit merged 2 commits intomainfrom Feb 20, 2025
Merged
Conversation
Reviewer's Guide by SourceryThis pull request addresses issue #237 by refactoring the Sequence diagram for renaming a chatsequenceDiagram
participant User
participant _ChatPageState
participant ChatHistoryItem
participant _historyRN
participant Storage
User->>_ChatPageState: Taps 'Rename Chat'
activate _ChatPageState
_ChatPageState->>_ChatPageState: Shows dialog to enter new title
User->>_ChatPageState: Enters new title
_ChatPageState->>ChatHistoryItem: copyWith(name: title)
activate ChatHistoryItem
ChatHistoryItem-->>_ChatPageState: Returns new ChatHistoryItem
deactivate ChatHistoryItem
_ChatPageState->>ChatHistoryItem: save()
activate ChatHistoryItem
ChatHistoryItem-->>_ChatPageState: void
deactivate ChatHistoryItem
_ChatPageState->>_historyRN: notify()
activate _historyRN
_historyRN-->>_ChatPageState: void
deactivate _historyRN
_ChatPageState->>Storage: _storeChat(chatId)
activate Storage
Storage-->>_ChatPageState: void
deactivate Storage
deactivate _ChatPageState
Sequence diagram for deleting a chat itemsequenceDiagram
participant User
participant _ChatPageState
participant chatItems
participant _historyRN
participant Storage
User->>_ChatPageState: Taps 'Delete Chat'
activate _ChatPageState
_ChatPageState->>_ChatPageState: Shows confirmation dialog
User->>_ChatPageState: Confirms deletion
_ChatPageState->>chatItems: remove(chatItem)
activate chatItems
chatItems-->>_ChatPageState: void
deactivate chatItems
_ChatPageState->>Storage: _storeChat(chatId)
activate Storage
Storage-->>_ChatPageState: void
deactivate Storage
_ChatPageState->>_historyRN: notify()
activate _historyRN
_historyRN-->>_ChatPageState: void
deactivate _historyRN
_ChatPageState->>_ChatPageState: _chatRN.notify()
deactivate _ChatPageState
Sequence diagram for generating chat titlesequenceDiagram
participant _Req
participant Completer
participant Loggers
participant ChatHistoryItem
participant _historyRN
participant _appbarTitleVN
_Req->>Completer: Completer()
activate Completer
Completer-->>_Req: completer
deactivate Completer
_Req->>Loggers: Loggers.app.warning('Gen title: $e')
activate Loggers
Loggers-->>_Req: void
deactivate Loggers
_Req->>_historyRN: _historyRN.notify()
activate _historyRN
_historyRN-->>_Req: void
deactivate _historyRN
_Req->>Completer: completer.complete()
activate Completer
Completer-->>_Req: void
deactivate Completer
_Req->>ChatHistoryItem: copyWith(name: title)
activate ChatHistoryItem
ChatHistoryItem-->>_Req: ne
deactivate ChatHistoryItem
_Req->>ChatHistoryItem: ne.save()
activate ChatHistoryItem
ChatHistoryItem-->>_Req: void
deactivate ChatHistoryItem
_Req->>_historyRN: _historyRN.notify()
activate _historyRN
_historyRN-->>_Req: void
deactivate _historyRN
_Req->>_appbarTitleVN: _appbarTitleVN.value = title
activate _appbarTitleVN
_appbarTitleVN-->>_Req: void
deactivate _appbarTitleVN
Updated class diagram for MarkdownCopyPageclassDiagram
class _MarkdownCopyPage {
build(BuildContext context) Widget
}
class Scaffold {
appBar: CustomAppBar
body: FutureWidget
}
class CustomAppBar {
title: Text
centerTitle: bool
}
class FutureWidget {
future: Future<String>
error: (e, s) => SimpleMarkdown
success: _buildBody
}
class SimpleMarkdown {
data: String
}
_MarkdownCopyPage -- Scaffold : contains
Scaffold -- CustomAppBar : has
Scaffold -- FutureWidget : has
FutureWidget -- SimpleMarkdown : on error
Updated class diagram for HistoryPageclassDiagram
class _HistoryPageState {
_buildHistoryListItem(String chatId) Widget
}
class ListTile {
title: Text
subtitle: ListenBuilder
}
class Text {
String data
TextOverflow overflow
TextStyle style
}
class ListenBuilder {
listenable: _timeRN
builder: (BuildContext context, Widget child) => Widget
}
_HistoryPageState -- ListTile : contains
ListTile -- Text : has title
ListTile -- ListenBuilder : has subtitle
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @lollipopkit - I've reviewed your changes - here's some feedback:
Overall Comments:
- It looks like you're removing
_historyRNMapand replacing it with a single_historyRN- does this mean only one history item can be updated at a time? - Consider extracting the title update logic into a separate function to improve readability.
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #237
Summary by Sourcery
This pull request fixes a bug where the raw markdown page was missing a title bar on macOS. It also improves the performance of the history page by using a simple Text widget instead of a ListenBuilder with an RNode for displaying the title, and by using a single RNode instance for history updates instead of multiple instances.
Bug Fixes:
Enhancements: