Repository navigation
feat: animate chat input icons on focus - #72
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 chat input gains focus, the three right-side icons (paperclip, eye, image) collapse and fade out, giving the TextInput the full pill width for typing. On blur they smoothly restore. The send/stop/mic button remains always visible so users can send while typing. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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 enhances the chat input user experience by introducing a dynamic animation for the accessory icons, allowing for a more expansive typing area when the input is active. Concurrently, it refines the application's approach to requesting notification permissions on Android, ensuring that users are prompted contextually and only when necessary, specifically before their initial download. 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
|
![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 introduces a nice animation to the chat input, collapsing the side icons on focus to provide more typing space. The implementation using React Native's Animated API is well-executed. I've left one minor suggestion to improve maintainability by replacing a magic number with constants.
Additionally, the refactoring of the notification permission logic is a great improvement. Moving it from App.tsx to the ModelsScreen and adding a user-friendly rationale dialog before the system prompt is a much better user experience.
| <Animated.View | ||
| pointerEvents={isFocused ? 'none' : 'auto'} | ||
| style={[styles.pillIcons, { | ||
| width: focusAnim.interpolate({ inputRange: [0, 1], outputRange: [108, 0] }), |
There was a problem hiding this comment.
The hardcoded value 108 for the width animation is a magic number. It's derived from the width of the three icons (3 * 36px). To improve maintainability, consider defining constants for these values outside the component and using them here. This will make it easier to update if an icon is added/removed or the icon size changes.
For example:
const PILL_ICON_WIDTH = 36; // From styles.ts pillIconButton.width
const NUM_PILL_ICONS = 3;
const ICONS_CONTAINER_WIDTH = PILL_ICON_WIDTH * NUM_PILL_ICONS;
// ... inside the component's style
width: focusAnim.interpolate({ inputRange: [0, 1], outputRange: [ICONS_CONTAINER_WIDTH, 0] }),There was a problem hiding this comment.
Good call — extracted PILL_ICON_SIZE and PILL_ICONS_WIDTH constants in styles.ts and used them both in the style definition and the animation interpolation. Fixed in 39278a3.
PermissionsAndroid.request() was passed a rationale object which caused Android to show its own alert dialog before the OS system prompt. Since we already show a custom bottom sheet rationale, this intermediate dialog was redundant — resulting in 3 dialogs for a single permission. Now the flow is: bottom sheet rationale → OS system dialog (2 steps). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace hardcoded 108 with PILL_ICONS_WIDTH (PILL_ICON_SIZE * 3) derived from the same constant used in the style definition. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Extract notification rationale logic into useNotifRationale hook to bring useModelsScreen under the 250-line function limit - Remove unused backgroundDownloadService import from App.tsx - Remove unused logger import from useModelsScreen.ts Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a nice animation for the chat input icons on focus, improving the user experience by providing more space for typing. The implementation using Animated.View is well-done. Additionally, the refactoring of the notification permission request to be more contextual is a great improvement. I have one suggestion to improve maintainability by extracting hardcoded animation durations into constants.
| const handleFocus = useCallback(() => { | ||
| setIsFocused(true); | ||
| Animated.timing(focusAnim, { toValue: 1, duration: 180, useNativeDriver: false }).start(); | ||
| }, [focusAnim]); | ||
|
|
||
| const handleBlur = useCallback(() => { | ||
| setIsFocused(false); | ||
| Animated.timing(focusAnim, { toValue: 0, duration: 200, useNativeDriver: false }).start(); | ||
| }, [focusAnim]); |
There was a problem hiding this comment.
The animation durations 180 and 200 are hardcoded. To improve maintainability and consistency, it's better to extract these magic numbers into named constants.
For example, you could define them in src/components/ChatInput/styles.ts and import them:
// in styles.ts
export const ANIMATION_DURATION_IN = 180;
export const ANIMATION_DURATION_OUT = 200;Then use them here:
// in index.tsx
import { ..., ANIMATION_DURATION_IN, ANIMATION_DURATION_OUT } from './styles';
// ...
Animated.timing(focusAnim, { toValue: 1, duration: ANIMATION_DURATION_IN, useNativeDriver: false }).start();
// ...
Animated.timing(focusAnim, { toValue: 0, duration: ANIMATION_DURATION_OUT, useNativeDriver: false }).start();There was a problem hiding this comment.
Done — extracted ANIM_DURATION_IN (180ms) and ANIM_DURATION_OUT (200ms) constants in styles.ts.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Width collapse alone just clips the icons. Adding translateX pushes them rightward so they visually slide out of the pill on focus. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Drop the translateX approach — it fights with overflow:hidden on the pill. Width collapse from PILL_ICONS_WIDTH→0 with a fast opacity fade (0→0.4 of the animation) gives a clean disappearing effect. The pill's overflow:hidden handles clipping naturally. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
/gemini review |
ChatInput (7 new): - Animated.timing fires on focus/blur - pointerEvents disabled/re-enabled on focus/blur - Icons accessible when unfocused - Send and stop buttons remain visible during focus useNotifRationale (7 new): - Skips rationale when not first download, on iOS, on API < 33 - Skips when permission already granted - Shows rationale on Android 33+ first download - Allow triggers permission request then proceeds - Dismiss proceeds without requesting permission Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Code Review
The pull request successfully implements the animation for chat input icons and introduces a notification rationale flow for Android 13+. The animation logic is sound, using Animated.timing with useNativeDriver: false to handle width changes. The notification rationale improves UX by explaining why permissions are needed before showing the system prompt. However, there is a minor UX issue where the rationale alert can appear multiple times if the user hasn't completed their first download yet. Additionally, a suggestion was made to ensure overflow: 'hidden' is applied directly to an Animated.View for safer clipping.
| const maybeShowNotifRationale = useCallback(async (proceed: () => void) => { | ||
| if (Platform.OS !== 'android' || Platform.Version < 33 || !isFirstDownload) { | ||
| proceed(); | ||
| return; | ||
| } | ||
| const alreadyGranted = await PermissionsAndroid.check( | ||
| PermissionsAndroid.PERMISSIONS.POST_NOTIFICATIONS, | ||
| ); | ||
| if (alreadyGranted) { | ||
| proceed(); | ||
| return; | ||
| } | ||
| pendingDownload.current = proceed; | ||
| setShowNotifRationale(true); | ||
| }, [isFirstDownload]); |
There was a problem hiding this comment.
The rationale alert will be shown every time a download is initiated as long as isFirstDownload remains true (i.e., until the first model is fully downloaded and added to the list). If a user starts multiple downloads in quick succession or dismisses the rationale once, they will be repeatedly prompted. Consider using a useRef or state variable to track if the rationale has already been shown in the current session to avoid redundant prompts.
There was a problem hiding this comment.
Good catch — added a hasShownRationale ref that gets set to true on first show. Subsequent calls skip the rationale and proceed directly. Fixed in 299e6e0.
| <Animated.View | ||
| pointerEvents={isFocused ? 'none' : 'auto'} | ||
| style={[styles.pillIcons, { | ||
| width: focusAnim.interpolate({ inputRange: [0, 1], outputRange: [PILL_ICONS_WIDTH, 0] }), | ||
| opacity: focusAnim.interpolate({ inputRange: [0, 0.4], outputRange: [1, 0], extrapolate: 'clamp' }), | ||
| }]} | ||
| > |
There was a problem hiding this comment.
While overflow: 'hidden' is applied to the parent pill container, it is safer to also apply it directly to this Animated.View. This ensures that as the width collapses to 0, the icon buttons (which have a fixed width of 36) are clipped immediately and don't potentially overlap with the TextInput or other elements if the parent's clipping doesn't align perfectly with the shrinking container.
<Animated.View
pointerEvents={isFocused ? 'none' : 'auto'}
style={[
styles.pillIcons,
{
width: focusAnim.interpolate({ inputRange: [0, 1], outputRange: [PILL_ICONS_WIDTH, 0] }),
opacity: focusAnim.interpolate({ inputRange: [0, 0.4], outputRange: [1, 0], extrapolate: 'clamp' }),
overflow: 'hidden'
}
]}
>
There was a problem hiding this comment.
Added overflow: 'hidden' directly on the icons Animated.View for safer clipping. Fixed in 299e6e0.
… clip - Add hasShownRationale ref to prevent repeated prompts when user dismisses and starts another download before the first completes - Add overflow:'hidden' directly on the icons Animated.View for safer clipping during width collapse - Add test for once-per-session behavior Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Icons now hide when user types any character and reappear when input is empty, providing a more intuitive experience than focus-based animation. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #72 +/- ##
==========================================
+ Coverage 87.95% 88.00% +0.04%
==========================================
Files 149 150 +1
Lines 6436 6479 +43
Branches 1784 1790 +6
==========================================
+ Hits 5661 5702 +41
- Misses 411 413 +2
Partials 364 364
🚀 New features to boost your workflow:
|
Summary
Type of Change
Screenshots / Screen Recordings
Android
iOS
Checklist
General
Testing
npm test)React Native Specific
project.pbxproj)SPACING/TYPOGRAPHYconstants from the themeuseThemedStylespattern (not inline or staticStyleSheet.create)FlatList/FlashList(not.map()insideScrollView)Security
Additional Notes
Animated.ValuewithuseNativeDriver: falseto animate both width (108→0) and opacity (1→0) on the icons container. Width animation is required to actually reclaim flex layout space (translateX alone only moves visually).pointerEvents='none'prevents hidden icons from intercepting touches during/after fade-out.overflow: 'hidden'on the pill clips icon content cleanly as the container width shrinks.useCallbackwraps focus/blur handlers to avoid unnecessary re-renders.