NostrContacts: fix layout and multiple bugs#3774
NostrContacts: fix layout and multiple bugs#3774myxmaster wants to merge 2 commits intoZeusLN:masterfrom
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the user experience and stability of the NostrContacts screen. It addresses several layout inconsistencies and visual bugs, particularly improving how contact rows are displayed and selected. A key architectural change involves leveraging native layout animations for smoother UI transitions and refining the back navigation flow to be more intuitive and robust. Additionally, a critical bug affecting contact selection logic has been resolved, ensuring accurate identification of contacts. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request provides a solid set of fixes and refactorings for the NostrContacts screen. The move from Animated to LayoutAnimation is a good simplification, and the layout cleanup removes previous hacks, improving maintainability. The fix for contact selection using npub instead of banner is a critical correction. Overall, the changes enhance both the user experience and the code quality. I have one suggestion regarding the FlatList implementation to ensure robustness.
a47a1f6 to
1518e91
Compare
|
ah ok, turns out this animation was never visible on android (even in a compiled release build, so not just a debugging performance issue). so i went back to |
still can't see that transition in both android and iOS |
1518e91 to
eff8917
Compare
|
during debugging i cannot see any animation as well. try using but i did change |
eff8917 to
629de3c
Compare
|
(rebased) |
|
@myxmaster please rebase again |
629de3c to
1db2be5
Compare
|
done |
changing it to 2000 does make it visible while transition but its not smooth |
1db2be5 to
eb0dae3
Compare
|
Alright, I now tested on a physical device (not emulator). Turns out the original implementation was already problematic on older hardware (noticeable latency when entering selection mode and when tapping individual checkboxes). Clean fix: Extracted Please test again. |
Description
Multiple layout and bug fixes:
UsingLayoutAnimationinstead ofAnimated/Easing(lets the OS handle layout transitions natively, no manual width interpolation needed)themeColor('highlight')SelectButtonicon in header usesthemeColor, correct height and is vertically centeredCloseicon in header removed ->Backicon (and hardware back button) now handle everything: first clear loaded data, then navigate backnpubinstead ofbanner(before: when you selected a contact without banner, all contacts without banner were selected)Please test if behavior is fine on iOS!
This pull request is categorized as a:
Checklist
yarn run tscand made sure my code compiles correctlyyarn run lintand made sure my code didn’t contain any problematic patternsyarn run prettierand made sure my code is formatted correctlyyarn run testand made sure all of the tests passTesting
If you modified or added a utility file, did you add new unit tests?
I have tested this PR on the following platforms (please specify OS version and phone model/VM):
I have tested this PR with the following types of nodes (please specify node version and API version where appropriate):
Locales
Third Party Dependencies and Packages
yarnafter this PR is merged inpackage.jsonandyarn.lockhave been properly updatedOther: