fix: v0.9.2 launch review issues #7898
Merged
Conversation
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
Reviewer's GuideThis pull request addresses several issues identified during the v0.9.2 launch review. It refines chat message loading and animations by implementing debounced loading for previous messages and adjusting scroll behavior. File block insertion in the editor is improved to replace empty paragraphs directly. Navigation is enhanced by automatically switching to a view's parent space upon opening. A UI glitch causing opacity on the locked page icon has been fixed. Finally, iOS pod dependencies have been updated. Class Diagram: Key Changes in v0.9.2 Launch Review FixesclassDiagram
class ChatAnimatedListState {
+Debounce _loadPreviousMessagesDebounce
+initState() void
+build(BuildContext context) Widget
+dispose() void
+_handleLoadPreviousMessages() void
+_onInserted(int position, Message data) void
+_handleToggleScrollToBottom() void
}
note for ChatAnimatedListState "Manages the animated list of chat messages.\n- Added Debounce _loadPreviousMessagesDebounce for fetching older messages.\n- initState() registers a listener to trigger _handleLoadPreviousMessages on scroll.\n- build() logic updated for initial scroll position and spacing.\n- _onInserted() updated for item insertion animation.\n- _handleToggleScrollToBottom() logic refined for showing/hiding scroll button."
class EditorStateExtension {
<<extension>>
+insertEmptyFileBlock(GlobalKey key) Future~void~
}
note for EditorStateExtension "Extension on EditorState for file block operations.\n- insertEmptyFileBlock() logic modified to replace an empty paragraph node with the file block, or insert it after the current node."
class DesktopHomeScreen {
+build(BuildContext context) Widget
-_switchToSpace(ViewPB view) Future~void~
}
note for DesktopHomeScreen "Main screen for desktop.\n- When a view is opened, it now calls _switchToSpace to potentially switch to the view's parent space.\n- Uses ViewBackendService to get view ancestors and switchToSpaceIdNotifier to trigger the space switch."
class _SpaceState {
+initState() void
+dispose() void
-_switchToSpace() void
}
note for _SpaceState "State for a space item in the sidebar.\n- initState() now listens to switchToSpaceIdNotifier.\n- _switchToSpace() is called when the notifier fires, triggering a SpaceBloc event to open the specified space."
class LockPageButtonWrapper {
+build(BuildContext context) Widget
}
note for LockPageButtonWrapper "Wrapper for the lock page button.\n- Removed Opacity widget to fix UI glitch where icon was semi-transparent when locked."
class GlobalNotifiers {
<<New>>
+ValueNotifier~ViewPB?~ switchToSpaceIdNotifier
}
note for GlobalNotifiers "New global ValueNotifier.\n- switchToSpaceIdNotifier is used to communicate the need to switch to a specific space (identified by ViewPB) from DesktopHomeScreen to _SpaceState."
ChatAnimatedListState ..> Debounce : uses
ChatAnimatedListState ..> Message : uses
DesktopHomeScreen ..> GlobalNotifiers : uses switchToSpaceIdNotifier
DesktopHomeScreen ..> ViewPB : uses
_SpaceState ..> GlobalNotifiers : listens to switchToSpaceIdNotifier
class Debounce {
+Debounce(Duration duration)
+call(Function callback) void
}
class ViewPB {
+String id
+isSpace() bool
}
class Message {
// Message properties
}
File-Level Changes
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 @LucasXu0 - I've reviewed your changes - here's some feedback:
_oldListinChatAnimatedListStatemay become outdated after inserts/removals, affecting subsequent_onUpdateddiffs.- Please clarify how list animations in
ChatAnimatedListStateare handled after_onInserted/_onRemoved/_onMovedcalls were removed from the event stream. - The
_onInsertedmethod inChatAnimatedListStateuses a hardcoded index 0 forinsertItemwhile its logic also depends on thepositionargument; this seems inconsistent.
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.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Feature Preview
PR Checklist
Summary by Sourcery
Address various UI and user experience issues in the AppFlowy application, focusing on chat, document editing, and workspace navigation
New Features:
Bug Fixes:
Enhancements:
Tests: