Repository navigation
feat: notif permission rationale + chat input icon animation - #71
alichherawalla wants to merge 7 commits into
Conversation
On Android 13+, intercept the first download attempt on the Models screen to explain why notification permission is needed and that it is optional. The OS dialog only appears if the user taps Allow; either way the download proceeds unblocked. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
When the text input gains focus, the paperclip/eye/image icons slide right and fade out (200ms) while their width collapses to 0 so the input expands to fill the freed space. Reverses on blur. Respects reduced-motion accessibility setting.
Summary of ChangesHello @alichherawalla, 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 by introducing a dynamic animation for the chat input, allowing more space for typing when focused. Concurrently, it improves the handling of Android notification permissions for downloads, making the process more transparent and user-friendly by prompting the user with a clear rationale before requesting the permission. Highlights
Changelog
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
|
Covers maybeShowNotifRationale platform/version/first-download/permission gates, handleNotifRationaleAllow (with and without rejection), dismiss, and the image model download path. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Code Review
The pull request successfully implements a smooth slide-and-fade animation for the chat input icons, improving the user experience by maximizing space for text entry. It also refactors the Android notification permission request to a rationale-based flow in the Models screen. The implementation is generally solid, but there are opportunities to improve performance through stable hook references and to ensure the notification rationale logic doesn't inadvertently exclude existing users.
I am having trouble creating individual review comments. Click here to see my feedback.
App.tsx (78-80)
Removing the notification permission request from the app startup is a good UX improvement. However, the new logic in ModelsScreen only prompts users on their 'first download'. This means existing users who already have models will never be prompted for this permission unless they delete their models or manually find the setting. Consider a more inclusive check for the rationale.
src/components/ChatInput/index.tsx (1)
The useCallback hook is needed for the suggested performance improvements in the animation handlers.
import React, { useState, useRef, useCallback } from 'react';
src/components/ChatInput/index.tsx (56)
The ICONS_WIDTH constant is currently defined inside the component. Since it is a static value derived from the icon dimensions (3 icons * 36px), it should ideally be defined outside the component to avoid recreation on every render and to serve as a single source of truth for the animation logic.
src/components/ChatInput/index.tsx (68-80)
These animation handlers are recreated on every render of the ChatInput component (which happens on every keystroke). Wrapping them in useCallback will provide stable references to the TextInput props and prevent unnecessary overhead.
const handleInputFocus = useCallback(() => {
const duration = reducedMotion ? 0 : 200;
iconsTranslateX.value = withTiming(ICONS_WIDTH, { duration });
iconsOpacity.value = withTiming(0, { duration });
iconsWidth.value = withTiming(0, { duration });
}, [reducedMotion, iconsTranslateX, iconsOpacity, iconsWidth]);
const handleInputBlur = useCallback(() => {
const duration = reducedMotion ? 0 : 250;
iconsTranslateX.value = withTiming(0, { duration });
iconsOpacity.value = withTiming(1, { duration });
iconsWidth.value = withTiming(ICONS_WIDTH, { duration });
}, [reducedMotion, iconsTranslateX, iconsOpacity, iconsWidth]);
src/screens/ModelsScreen/useModelsScreen.ts (135-136)
The isFirstDownload check restricts the notification rationale to only new users. Existing users who already have models will skip this logic and never be prompted for the permission that was removed from App.tsx. It would be better to check if the permission has been requested before or simply check the current permission status using PermissionsAndroid.check without the isFirstDownload constraint.
src/screens/ModelsScreen/useModelsScreen.ts (153-165)
These callbacks are recreated on every render because they depend on the text and image objects, which are themselves recreated on every render of useModelsScreen. This can lead to unnecessary re-renders of the tab components. Consider stabilizing the objects returned by useTextModels and useImageModels using useMemo within those hooks.
…hatInput Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #71 +/- ##
==========================================
- Coverage 87.95% 87.89% -0.06%
==========================================
Files 149 149
Lines 6436 6479 +43
Branches 1784 1789 +5
==========================================
+ Hits 5661 5695 +34
- Misses 411 419 +8
- Partials 364 365 +1
🚀 New features to boost your workflow:
|
…liable icon hide on focus - overflow: 'hidden' is not an animatable property and caused the animated style block to not apply on some RN versions - translateX on the same view as the overflow clip does not work correctly (transform is applied after layout clipping) - Move overflow: 'hidden' to the static pillIcons style - Animate only width (collapse) + opacity (fade) which are reliable in Reanimated flex layouts - Add 4 unit tests covering focus/blur animation with and without reduced motion Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- App.tsx: remove unused backgroundDownloadService import (fixes lint error) - ChatInput: move ICONS_WIDTH outside component; add useCallback to handleInputFocus/handleInputBlur with stable deps - useModelsScreen: drop isFirstDownload guard from maybeShowNotifRationale so existing users who revoke permission are also prompted; reference specific handler fns (textHandleDownload/imageHandleDownload) in useCallback deps instead of entire hook return objects Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
/gemini review |
There was a problem hiding this comment.
Code Review
The pull request introduces two main features: a notification permission rationale modal for Android 13+ and an animation for chat input icons on focus/blur. The changes are well-implemented, with appropriate use of React hooks and reanimated for the animation. The notification rationale logic correctly handles platform versions and permission states, and the associated unit tests provide good coverage. The animation logic also correctly respects reduced motion settings. Overall, the code maintains good quality and clarity.
… test - ChatInput: restore iconsTranslateX shared value; on focus slides icons right (ICONS_WIDTH) + fades + collapses width; on blur reverses all three. pillIcons overflow set to 'visible' so the pill's overflow:hidden does the clipping as icons translate past the right edge. - notifRationale test: update 'not first download' case to reflect the new behaviour — existing users with permission not granted now see the rationale modal instead of being silently passed through. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Addressed all Gemini review comments — summary of changes across the two fix commits ( App.tsx — removed unused ChatInput — moved useModelsScreen — removed the notifRationale test — updated the "not first download" test case to reflect the new intended behaviour: models already downloaded + permission not granted → modal still shown. |
|



What
Two improvements shipped together:
1. Notification permission rationale modal (Android 13+)
On Android 13+ (API 33), instead of silently requesting
POST_NOTIFICATIONSat app launch, an in-app rationale modal now shows the first time a user tries to download a model from the Models screen.2. Chat input icon animation on focus
When the chat input gains focus the three right-side icons (paperclip, eye, image) slide off to the right and fade out, freeing up the full pill width for typing. On blur they slide back in.
Why
Rationale modal: The previous approach fired the OS permission dialog cold on startup with no context. This gives users a clear explanation before the OS dialog, improving transparency and reducing dismissal rates.
Icon animation: Maximises the input area when the user is actively typing without removing the icons entirely.
How
Rationale modal:
requestNotificationPermission()call fromApp.tsxmaybeShowNotifRationale()inuseModelsScreenthat intercepts the first download attempt when: platform is Android 13+, no models downloaded yet, permission not already grantedIcon animation:
useIconsAnimationhook driving three Reanimated shared values:iconsWidth,iconsTranslateX,iconsOpacityuseReducedMotion()— instant when reduced motion is enabledTest plan