Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request successfully removes the 200-character limit from the MemoryDialog by enabling multi-line input. However, this change introduces a critical regression by removing the UI for selecting memory visibility. As a result, new memories are always created as 'private', and it's no longer possible to edit a memory's visibility through this dialog. I've left a specific comment on this issue. Furthermore, another part of the app for editing memories, MemoryEditSheet, was not updated, so it still enforces a 200-character limit, creating an inconsistent user experience.
| child: ElevatedButton( | ||
| onPressed: () { | ||
| if (contentController.text.trim().isNotEmpty) { | ||
| if (widget.memory != null) { | ||
| widget.provider.editMemory(widget.memory!, contentController.text); | ||
| MixpanelManager().memoriesPageEditedMemory(); | ||
| } else { | ||
| widget.provider | ||
| .createMemory(contentController.text, MemoryVisibility.private, MemoryCategory.interesting); | ||
| MixpanelManager().memoriesPageCreatedMemory(MemoryCategory.interesting); | ||
| } | ||
| Navigator.pop(context); | ||
| } | ||
| }, | ||
| style: ElevatedButton.styleFrom( | ||
| backgroundColor: Colors.deepPurpleAccent, | ||
| foregroundColor: Colors.white, | ||
| padding: const EdgeInsets.symmetric(vertical: 14), | ||
| shape: RoundedRectangleBorder( | ||
| borderRadius: BorderRadius.circular(12), | ||
| ), | ||
| child: Row( | ||
| mainAxisSize: MainAxisSize.min, | ||
| children: [ | ||
| Icon( | ||
| Icons.keyboard_return, | ||
| size: 13, | ||
| color: Colors.grey.shade400, | ||
| ), | ||
| const SizedBox(width: 4), | ||
| Text( | ||
| 'Press done to save', | ||
| style: TextStyle( | ||
| color: Colors.grey.shade400, | ||
| fontSize: 11, | ||
| ), | ||
| ), | ||
| ], | ||
| ), | ||
| ), | ||
| Text( | ||
| '${contentController.text.length}/200', | ||
| style: TextStyle( | ||
| color: Colors.grey.shade500, | ||
| fontSize: 11, | ||
| child: Text( | ||
| widget.memory != null ? 'Update Memory' : 'Save Memory', | ||
| style: const TextStyle( | ||
| fontSize: 16, | ||
| fontWeight: FontWeight.w600, | ||
| ), | ||
| ), | ||
| ], | ||
| ), |
There was a problem hiding this comment.
This refactoring removes the ability for users to control a memory's visibility, which is a significant functional regression.
- Creating memories: New memories are now hardcoded to
privatevisibility (line 117). Previously, they defaulted topublic, and the user could choose a different visibility. - Editing memories: The option to change an existing memory's visibility has been removed from this dialog.
This change is not mentioned in the pull request description. If this loss of functionality is unintentional, the visibility controls should be restored. If it is intentional, this breaking change should be clearly communicated to users.
1 participant
#3460