feat: update Servers history and Servers List layout#6733
feat: update Servers history and Servers List layout#6733diegolmello merged 49 commits intodevelopfrom
Conversation
WalkthroughThis PR replaces long-press deletion with swipe-to-delete for server items, adds swipeable components and animated delete actions, updates ServerItem and ServersHistory UI, persists iconURL in server history with schema/migration, updates sagas to populate iconURL, revises E2E tests for swipe flows, and adds i18n keys across locales. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Gesture as Pan Gesture Handler
participant SwipeUI as SwipeableDeleteTouchable
participant Anim as Reanimated
participant Action as DeleteAction
participant App as App Callback
User->>Gesture: Swipe item (left/right per locale)
Gesture->>Anim: update transX (shared value)
Anim->>SwipeUI: animate item position
Anim->>Action: reveal delete button (parallax/threshold)
alt transX crosses longSwipe threshold
Anim->>App: runOnJS -> onDeletePress (auto)
App->>SwipeUI: remove item
else user taps revealed delete button
User->>Action: tap delete
Action->>App: onDeletePress
App->>SwipeUI: remove item
end
SwipeUI->>Anim: spring transX to closed state (if not deleted)
SwipeUI->>User: UI updated (deleted or closed)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45–75 minutes Areas to focus:
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
OtavioStasiak
had a problem deploying
to
approve_e2e_testing
GitHub Actions
Error
OtavioStasiak
had a problem deploying
to
approve_e2e_testing
GitHub Actions
Error
OtavioStasiak
had a problem deploying
to
approve_e2e_testing
GitHub Actions
Error
OtavioStasiak
had a problem deploying
to
approve_e2e_testing
GitHub Actions
Error
OtavioStasiak
temporarily deployed
to
approve_e2e_testing
GitHub Actions
Inactive
OtavioStasiak
had a problem deploying
to
experimental_android_build
GitHub Actions
Error
OtavioStasiak
had a problem deploying
to
official_android_build
GitHub Actions
Error
OtavioStasiak
had a problem deploying
to
experimental_ios_build
GitHub Actions
Error
OtavioStasiak
had a problem deploying
to
official_ios_build
GitHub Actions
Error
OtavioStasiak
temporarily deployed
to
approve_e2e_testing
GitHub Actions
Inactive
OtavioStasiak
had a problem deploying
to
official_android_build
GitHub Actions
Error
OtavioStasiak
had a problem deploying
to
experimental_android_build
GitHub Actions
Error
OtavioStasiak
had a problem deploying
to
experimental_ios_build
GitHub Actions
Error
OtavioStasiak
had a problem deploying
to
official_ios_build
GitHub Actions
Error
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (5)
app/i18n/locales/nn.json (1)
390-390: Nynorsk form likely needed instead of Bokmål."Arbeidsområder" is Bokmål; for nn (Nynorsk) consider "Arbeidsområde" (plural indefinite) or "Arbeidsområda" (definite plural), per project style.
app/i18n/locales/zh-TW.json (1)
15-16: Wording tweak for zh‑TW: prefer 啟用 over 啟動.Use "啟用以選擇伺服器" for a UI activation. Keep "可用操作：刪除" as is.
Apply this diff:
- "Activate_to_select_server": "啟動以選擇伺服器", + "Activate_to_select_server": "啟用以選擇伺服器",app/i18n/locales/ja.json (1)
15-16: A11y keys added correctly.Japanese reads fine; optional alt “アクティブ化して…” not required.
app/i18n/locales/zh-CN.json (1)
15-16: 新增键值 OK。简体用词自然，句读与意图清晰。
If desired later, normalize mixed 繁/简 across zh-CN for consistency (not blocking).
app/views/NewServerView/components/ServersHistoryItem/styles.ts (1)
7-34: Consider sharing common styles with ServerItem.The styles defined here (container, serverIcon, textContainer, title, subtitle) are nearly identical to those in
app/containers/ServerItem/styles.ts. Consider extracting shared styles to reduce duplication and improve maintainability.For example, create a shared styles module:
// app/containers/ServerItem/sharedStyles.ts export const ROW_HEIGHT = 56; export const commonStyles = { container: { flexDirection: 'row', alignItems: 'center', padding: 12, minHeight: ROW_HEIGHT }, serverIcon: { width: 44, height: 44, borderRadius: 4 }, textContainer: { flex: 1, flexDirection: 'column', justifyContent: 'center', paddingRight: 18, paddingLeft: 12 } };Then import and reuse in both files.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (2)
app/containers/ServerItem/__snapshots__/ServerItem.test.tsx.snapis excluded by!**/*.snapapp/views/NewServerView/components/ServersHistoryItem/__snapshots__/ServersHistoryItem.test.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (49)
.maestro/tests/assorted/changeserver.yaml(6 hunks).maestro/tests/assorted/delete-server.yaml(1 hunks).maestro/tests/onboarding/server-history.yaml(2 hunks)app/containers/ServerItem/ServerItem.stories.tsx(2 hunks)app/containers/ServerItem/Touchable.tsx(1 hunks)app/containers/ServerItem/index.tsx(3 hunks)app/containers/ServerItem/styles.ts(2 hunks)app/containers/SwipeableDeleteItem/Actions.tsx(1 hunks)app/containers/SwipeableDeleteItem/Touchable.tsx(1 hunks)app/containers/SwipeableDeleteItem/index.ts(1 hunks)app/containers/Touch.tsx(2 hunks)app/definitions/IServerHistory.ts(1 hunks)app/i18n/locales/ar.json(2 hunks)app/i18n/locales/bn-IN.json(1 hunks)app/i18n/locales/cs.json(1 hunks)app/i18n/locales/de.json(1 hunks)app/i18n/locales/en.json(2 hunks)app/i18n/locales/es.json(2 hunks)app/i18n/locales/fi.json(1 hunks)app/i18n/locales/fr.json(1 hunks)app/i18n/locales/hi-IN.json(1 hunks)app/i18n/locales/hu.json(1 hunks)app/i18n/locales/it.json(2 hunks)app/i18n/locales/ja.json(2 hunks)app/i18n/locales/nl.json(2 hunks)app/i18n/locales/nn.json(1 hunks)app/i18n/locales/no.json(2 hunks)app/i18n/locales/pt-BR.json(2 hunks)app/i18n/locales/pt-PT.json(2 hunks)app/i18n/locales/ru.json(2 hunks)app/i18n/locales/sv.json(1 hunks)app/i18n/locales/ta-IN.json(1 hunks)app/i18n/locales/te-IN.json(1 hunks)app/i18n/locales/tr.json(2 hunks)app/i18n/locales/zh-CN.json(1 hunks)app/i18n/locales/zh-TW.json(1 hunks)app/lib/constants/colors.ts(1 hunks)app/lib/database/model/ServersHistory.js(1 hunks)app/lib/database/model/servers/migrations.js(1 hunks)app/lib/database/schema/servers.js(2 hunks)app/sagas/login.js(1 hunks)app/sagas/selectServer.ts(1 hunks)app/views/NewServerView/components/ServersHistoryActionSheetContent/index.tsx(2 hunks)app/views/NewServerView/components/ServersHistoryItem/ServersHistoryItem.stories.tsx(1 hunks)app/views/NewServerView/components/ServersHistoryItem/ServersHistoryItem.test.tsx(1 hunks)app/views/NewServerView/components/ServersHistoryItem/index.tsx(1 hunks)app/views/NewServerView/components/ServersHistoryItem/styles.ts(1 hunks)app/views/RoomsListView/components/ServersList.tsx(4 hunks)app/views/RoomsListView/styles.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (12)
app/views/NewServerView/components/ServersHistoryItem/styles.ts (2)
app/containers/ServerItem/index.tsx (1)
ROW_HEIGHT(11-11)app/views/NewServerView/components/ServersHistoryItem/index.tsx (1)
ROW_HEIGHT(11-11)
app/views/NewServerView/components/ServersHistoryItem/ServersHistoryItem.stories.tsx (4)
app/definitions/IServerHistory.ts (1)
TServerHistoryModel(11-11)app/theme.tsx (2)
TSupportedThemes(8-8)ThemeContext(18-18)app/views/NewServerView/components/ServersHistoryItem/index.tsx (1)
IServersHistoryItem(13-17)app/lib/constants/colors.ts (1)
themes(304-304)
app/views/NewServerView/components/ServersHistoryItem/ServersHistoryItem.test.tsx (1)
.rnstorybook/generateSnapshots.tsx (1)
generateSnapshots(10-22)
app/views/NewServerView/components/ServersHistoryItem/index.tsx (2)
app/definitions/IServerHistory.ts (1)
TServerHistoryModel(11-11)app/theme.tsx (1)
useTheme(29-29)
app/containers/SwipeableDeleteItem/Actions.tsx (3)
app/containers/SwipeableDeleteItem/index.ts (2)
IDeleteActionProps(4-4)DeleteAction(2-2)app/theme.tsx (1)
useTheme(29-29)app/containers/CustomIcon/index.tsx (1)
CustomIcon(38-38)
app/containers/ServerItem/Touchable.tsx (3)
app/containers/ServerItem/index.tsx (2)
IServerItemTouchableProps(13-13)ROW_HEIGHT(11-11)app/theme.tsx (1)
useTheme(29-29)app/containers/ServerItem/styles.ts (4)
ROW_HEIGHT(5-5)ACTION_WIDTH(6-6)LONG_SWIPE(8-8)SMALL_SWIPE(7-7)
app/sagas/selectServer.ts (1)
app/sagas/encryption.js (1)
serverInfo(27-27)
app/views/NewServerView/components/ServersHistoryActionSheetContent/index.tsx (1)
app/lib/constants/colors.ts (1)
colors(280-302)
app/containers/ServerItem/index.tsx (1)
app/theme.tsx (1)
useTheme(29-29)
app/containers/ServerItem/ServerItem.stories.tsx (3)
app/containers/ServerItem/index.tsx (1)
IServerItem(15-25)app/theme.tsx (2)
TSupportedThemes(8-8)ThemeContext(18-18)app/lib/constants/colors.ts (1)
themes(304-304)
app/sagas/login.js (1)
app/sagas/rooms.js (7)
serversCollection(16-16)serversCollection(40-40)serversDB(15-15)serversDB(32-32)server(36-36)serverRecord(18-18)serverRecord(42-42)
app/containers/SwipeableDeleteItem/Touchable.tsx (2)
app/theme.tsx (1)
useTheme(29-29)app/containers/SwipeableDeleteItem/Actions.tsx (1)
DeleteAction(31-109)
🔇 Additional comments (61)
app/i18n/locales/ta-IN.json (1)
862-862: Translation terminology improved.The "Workspaces" key has been updated from "பணிகள்" (jobs/tasks) to "வேலைத்தளங்கள்" (workspaces), providing more semantically accurate and contextually appropriate terminology for the application.
app/i18n/locales/bn-IN.json (1)
862-862: Localization update: Verify consistency across related workspace labels.The translation for "Workspaces" has been updated from a transliterated form ("ওয়ার্কস্পেস") to a more idiomatic Bengali term ("কর্মস্থানগুলি"). While this appears intentional and aligned with the PR's accessibility and layout improvements, note that other workspace-related keys in this file (e.g., lines 31, 40-41) still use the transliterated "ওয়ার্কস্পেস" or mixed "কর্মস্থান" forms.
Consider verifying that this translation choice is consistent with the broader localization strategy, particularly if other locale files follow a similar pattern for this PR.
app/views/RoomsListView/styles.ts (2)
38-42: LGTM! Consistent text styling.The
lineHeight: 24addition maintains a 1.5 ratio with thefontSize: 16, which is good for readability and is consistent with thegroupTitlestyle (lines 24-28).
43-49: LGTM! Button styling updated appropriately.The
paddingHorizontal: 16andborderRadius: 4values provide appropriate spacing and visual polish for the workspace creation button as part of the UI layout refresh.app/i18n/locales/it.json (1)
19-20: Locale entries verified consistent across all language files.The new accessibility labels (
Activate_to_select_serverandActivate_to_select_server_Available_actions_delete) have been added consistently across all 19 supported language files. TheWorkspacestranslation update in Italian ("Spazi di lavoro") is also reflected across all corresponding locale files. Changes are complete and properly synchronized.app/containers/Touch.tsx (4)
3-10: LGTM!The accessibility type imports are correctly added and properly structured.
18-20: LGTM!The interface extensions properly add accessibility support with correct types and maintain backward compatibility by making them optional.
26-41: LGTM!The prop destructuring correctly includes the new accessibility props with consistent formatting.
82-88: Verify nested accessibility behavior with screen readers.The accessibility props are applied to the inner
ViewwhileRectButtonis the touch target. This nested pattern can sometimes cause confusion or double announcements with screen readers.Please verify:
- Screen readers correctly announce the custom actions without duplicating announcements
- The
accessibleprop behavior aligns with expectations whenaccessibilityActionsare provided- Users can trigger both the default press action (via RectButton) and custom accessibility actions (via View) as intended
Consider explicitly setting
accessible={true}whenaccessibilityActionsis provided to ensure consistent behavior:<View - accessible={accessible} + accessible={accessible ?? (accessibilityActions && accessibilityActions.length > 0)} accessibilityLabel={accessibilityLabel} accessibilityHint={accessibilityHint} accessibilityActions={accessibilityActions} onAccessibilityAction={onAccessibilityAction} style={viewStyle}>app/i18n/locales/nl.json (2)
15-16: New accessibility labels for swipe-to-delete feature look good.The Dutch translations for the new server selection and delete action accessibility labels are grammatically correct and appropriately phrased for accessibility hints.
747-747: Verify the Dutch plural form for "Workspaces" — consider reverting to "werkruimten".The translation in nl.json uses "Werkruimtes" (line 747), which follows English pluralization conventions rather than standard Dutch grammar. The correct Dutch plural of "werkruimte" is "werkruimten." This is inconsistent with other Germanic languages in the codebase: German uses "Arbeitsbereiche" and Norwegian uses "Arbeidsområder"—both proper plurals. Either commit to this as intentional terminology or revert to "werkruimten" for grammatical correctness.
app/i18n/locales/hi-IN.json (1)
862-862: Verify localization consistency for "Workspaces" key.The value at line 862 is currently "वर्कस्पेसेस" (a transliteration of English "Workspaces"), while the same concept appears as "कार्यस्थान" (proper Hindi translation) in 24+ other keys throughout the file (e.g., lines 25, 26, 142, 160, and many others). This creates a localization inconsistency.
If this change is intentional for brand/UI consistency or accessibility requirements, please clarify in the commit message or comment. Otherwise, consider aligning with the existing Hindi translation used consistently elsewhere in the file.
app/i18n/locales/tr.json (1)
15-16: LGTM — clear and consistent Turkish translations.New activation strings and "Çalışma Alanları" look correct and consistent with other keys.
Also applies to: 637-637
app/i18n/locales/no.json (1)
21-22: LGTM — Bokmål translations read well.Activation hints and "Arbeidsområder" align with the rest of no.json.
Also applies to: 912-912
app/i18n/locales/en.json (1)
21-23: LGTM — keys and copy are consistent.Activation hints and "Workspaces" match current terminology and tone.
Also applies to: 966-966
app/i18n/locales/ru.json (1)
20-21: LGTM — natural Russian phrasing and consistent terminology.Both activation strings and “Рабочие пространства” align with existing ru entries.
Also applies to: 794-794
app/i18n/locales/pt-BR.json (1)
21-22: A11y additions OK.Wording is clear; consistent with other locales.
app/i18n/locales/ja.json (1)
534-534: Workspaces label OK.Matches existing terminology.
app/i18n/locales/fi.json (1)
20-21: Hyvä lisäys.Clear Finnish phrasing; consistent with other locales.
app/i18n/locales/sv.json (1)
20-21: Ser bra ut.Translations align with a11y copy in other locales.
app/i18n/locales/de.json (1)
21-22: Deutsch Übersetzungen passen.Grammar and casing consistent; no placeholders involved.
app/i18n/locales/ar.json (1)
15-16: Translations added successfully for new server selection UI features.The additions are syntactically correct, properly formatted JSON, and maintain alphabetical ordering within the locale file. The Arabic translations appropriately convey the activation prompts for server selection and the available delete action.
Also applies to: 607-607
app/i18n/locales/pt-PT.json (1)
15-16: Translations added successfully for Portuguese (Portugal) locale.The new keys are properly positioned alphabetically, syntactically correct, and the Portuguese translations accurately reflect the English counterparts for server selection activation and delete action availability.
Also applies to: 504-504
app/i18n/locales/es.json (1)
15-16: Translations added successfully for Spanish locale.The three new keys are correctly positioned in alphabetical order, syntactically valid JSON, and the Spanish translations appropriately convey the server selection activation prompts and delete action availability.
Also applies to: 441-441
app/i18n/locales/fr.json (1)
15-16: Translations added successfully for French locale.The new keys are properly alphabetized, syntactically correct, and the French translations accurately represent the server selection and delete action concepts for the new UI layout.
Also applies to: 747-747
app/lib/constants/colors.ts (1)
252-252: LGTM: Theme consistency improvement.The change aligns the black theme's
buttonBackgroundSecondaryDefaultwith the dark theme (Line 160), improving visual consistency across themes.app/sagas/selectServer.ts (1)
251-251: LGTM: Server history enriched with icon URL.The addition of
iconURLto server history entries aligns with the schema change and enables displaying server icons in the history UI. The optional nature of the field (as defined in the schema) correctly handles cases whereserverInfo.iconURLmay be undefined.app/definitions/IServerHistory.ts (1)
8-8: LGTM: Backward-compatible interface extension.The optional
iconURLproperty addition is backward compatible and aligns with the database schema changes.app/views/NewServerView/components/ServersHistoryItem/ServersHistoryItem.test.tsx (1)
1-4: LGTM: Standard Storybook snapshot testing.The test file follows the established pattern for generating snapshot tests from Storybook stories.
app/lib/database/model/ServersHistory.js (1)
15-15: LGTM: Database field mapping for icon URL.The field mapping correctly connects the TypeScript
iconURLproperty to theicon_urldatabase column, completing the data layer integration.app/views/NewServerView/components/ServersHistoryItem/ServersHistoryItem.stories.tsx (1)
1-79: LGTM: Comprehensive Storybook stories.The story file provides excellent coverage with variants for:
- Default content and edge cases (long text, missing icon)
- Swipe/delete interactions
- Theme variations (light, dark, black)
The structure is well-organized and the ThemeContext wrapper enables proper theme testing.
.maestro/tests/assorted/delete-server.yaml (1)
81-113: LGTM: E2E test updated for swipe-to-delete interaction.The test properly covers the new delete workflow:
- Swipe left to reveal delete button
- Tap the revealed delete button
- Confirm deletion in dialog
- Wait for animation to complete
- Verify the server item is removed from the list
The addition of
waitForAnimationToEndand thenotVisibleassertion ensure robust test behavior.app/lib/database/schema/servers.js (1)
4-4: Migration for schema version 17 verified and correctly configured.The verification confirms that the migration exists at line 160-167 of
app/lib/database/model/servers/migrations.js. It properly adds theicon_urlcolumn as a string type (optional) to theservers_historytable, matching the schema version bump.app/containers/ServerItem/ServerItem.stories.tsx (2)
21-21: LGTM! Prop rename fromonLongPresstoonDeletePressis consistent.The renaming aligns with the PR's transition to swipe-to-delete interactions, making the API clearer.
Also applies to: 27-27, 35-35
56-61: LGTM! Story updated to demonstrate swipe-to-delete functionality.The renamed
SwipeActionsstory with two examples effectively showcases the newonDeletePressbehavior.app/containers/ServerItem/styles.ts (4)
6-8: LGTM! Swipe gesture constants are well-defined.The constants
ACTION_WIDTH,SMALL_SWIPE, andLONG_SWIPEprovide clear thresholds for swipe-to-delete interactions.
36-41: Verify the positioning ofactionsLeftContainer.The
actionsLeftContainerspans fromleft: 0toright: 0, covering the full width. Confirm this is the intended behavior for the swipe-to-delete actions layout, as the name suggests a left-aligned container.
42-46: Incomplete positioning foractionRightButtonContainer.The style uses
position: 'absolute'andtop: 0, but lacksrightorleftpositioning. Verify that this style is completed elsewhere (e.g., dynamically applied) or if positioning should be added here.
47-52: LGTM! Action button dimensions matchACTION_WIDTH.The
actionButtonstyle correctly uses the exportedACTION_WIDTHconstant for consistent sizing.app/lib/database/model/servers/migrations.js (1)
159-167: LGTM! Database migration addsicon_urlcolumn to server history.The migration to version 17 correctly adds an optional
icon_urlcolumn to theservers_historytable, following established migration patterns and supporting the PR's iconURL feature.app/sagas/login.js (3)
95-95: LGTM! Servers collection reference added for iconURL lookup.The
serversCollectionis correctly initialized to access the servers table for fetching iconURL data.
101-108: LGTM! Safe iconURL retrieval with error handling.The try/catch block appropriately handles the case where the server record might not exist yet, ensuring the login flow continues even if iconURL retrieval fails.
113-115: LGTM! Conditional iconURL update ensures data integrity.The conditional check ensures
iconURLis only set when a valid value exists, preventing null or undefined values from being stored..maestro/tests/onboarding/server-history.yaml (2)
29-30: LGTM! Updated to use object-based selector.The change to an object-based
idselector for the server history item tap is consistent with the test framework's best practices.
48-56: LGTM! Swipe-to-delete flow correctly implemented.The test now validates the swipe-left gesture, waits for the animation to complete, and taps the revealed delete button. This accurately reflects the new swipe-to-delete interaction pattern.
app/containers/SwipeableDeleteItem/index.ts (1)
1-4: LGTM! Clean barrel export for SwipeableDeleteItem components.The file provides a well-organized public API by re-exporting the
SwipeableDeleteTouchableandDeleteActioncomponents along with their type definitions, following standard barrel export patterns.app/views/NewServerView/components/ServersHistoryItem/styles.ts (1)
5-5: LGTM!ROW_HEIGHTconstant matches ServerItem.The
ROW_HEIGHT = 56value is consistent withapp/containers/ServerItem/styles.ts, ensuring uniform row heights across server items and history items..maestro/tests/assorted/changeserver.yaml (3)
52-53: LGTM! Animation waits improve test stability.The added
waitForAnimationToEndsteps ensure UI animations complete before proceeding with subsequent actions, reducing flakiness in the test suite.Also applies to: 114-115, 128-129, 145-146, 187-188
74-75: Review the 5000ms animation timeout.The
waitForAnimationToEndtimeout of 5000ms appears excessive compared to the 500ms used elsewhere in this file. Verify if this longer timeout is necessary or if it can be reduced for faster test execution.
156-202: LGTM! Comprehensive swipe-to-delete test coverage.The new test sequence thoroughly validates the swipe-to-delete flow:
- Reveals delete button via swipe gesture
- Taps delete and confirms deletion
- Verifies alternate server removal
- Confirms main server preservation
This provides excellent end-to-end coverage of the swipe-to-delete feature.
app/containers/ServerItem/Touchable.tsx (1)
1-46: LGTM!This is a clean, straightforward wrapper component that properly forwards props to
SwipeableDeleteTouchable. The interface is well-defined, memoization is appropriate, and theming is correctly applied.app/views/RoomsListView/components/ServersList.tsx (3)
114-121: LGTM!The change from
onLongPresstoonDeletePressaligns with the PR's objective to replace long-press deletion with swipe-to-delete gestures.
126-133: LGTM!The UI updates—changing the background color to
surfaceLightand updating the header text to "Workspaces"—align with the new layout objectives.
143-153: LGTM!The button implementation is clean, properly themed, and includes appropriate test IDs and internationalization.
app/containers/SwipeableDeleteItem/Actions.tsx (1)
1-129: LGTM!This is a well-implemented animated delete action component with:
- Proper RTL/LTR support with distinct animation logic for each direction
- Smooth parallax effects using interpolation
- Haptic feedback on state transitions
- Good accessibility with labels and accessible flag
- Appropriate use of
runOnJSto bridge worklet and JS contextsapp/containers/ServerItem/index.tsx (2)
29-38: LGTM!Good accessibility implementation with context-aware labels and hints. The
serverNamefallback toitem.idensures meaningful labels even when the name is missing.
40-46: No changes needed; the fallback pattern is intentional and correct.The
IServerIteminterface declaresonDeletePress?(): voidas optional, but theISwipeableDeleteTouchablePropsinterface requiresonDeletePress(): voidas a non-null function. The fallback empty functiononDeletePress || (() => {})at line 42 is an intentional adapter that allows ServerItem to accept an optional prop while satisfying the child component's requirement. This pattern is correct and requires no action.app/views/NewServerView/components/ServersHistoryItem/index.tsx (1)
1-52: LGTM!This component follows the same pattern as
ServerItemwith appropriate adaptations for server history:
- Required
onDeletePress(history items are always deletable)- Proper accessibility with username fallback
- Consistent use of
ServerItemTouchablewrapper- Clean rendering of URL and username
app/views/NewServerView/components/ServersHistoryActionSheetContent/index.tsx (2)
13-26: LGTM!The header styles are well-defined and use
sharedStyles.textRegularfor consistency with the rest of the application.
43-57: LGTM!Good implementation with:
- Proper accessibility ordering using
A11y.OrderandA11y.Index- Correct callback wiring for
onPressandonDeletePress- Consistent "Workspaces" header matching
ServersList.tsx- Clean separation between items
app/containers/SwipeableDeleteItem/Touchable.tsx (1)
151-180: LGTM!The render implementation is well-structured with:
- Proper gesture detection wrapping
- DeleteAction correctly positioned behind content
- Clean animation application
- Good accessibility support with actions and labels
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
OtavioStasiak
had a problem deploying
to
approve_e2e_testing
GitHub Actions
Error
OtavioStasiak
had a problem deploying
to
official_android_build
GitHub Actions
Error
OtavioStasiak
had a problem deploying
to
experimental_ios_build
GitHub Actions
Error
OtavioStasiak
had a problem deploying
to
official_ios_build
GitHub Actions
Error
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
app/i18n/locales/nl.json (1)
15-16: Verify consistency across all locale files in the PR scope.Since the AI summary indicates that
Activate_to_select_serverandActivate_to_select_server_Available_actions_deletekeys are added across multiple locales (ar.json, de.json, es.json, etc.), ensure all translations follow similar patterns and are semantically consistent across languages.Also applies to: 747-747
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (21)
app/containers/ServerItem/Touchable.tsx(1 hunks)app/containers/SwipeableDeleteItem/Actions.tsx(1 hunks)app/i18n/locales/ar.json(2 hunks)app/i18n/locales/bn-IN.json(2 hunks)app/i18n/locales/cs.json(1 hunks)app/i18n/locales/de.json(1 hunks)app/i18n/locales/hi-IN.json(2 hunks)app/i18n/locales/hu.json(1 hunks)app/i18n/locales/ja.json(2 hunks)app/i18n/locales/nl.json(2 hunks)app/i18n/locales/nn.json(2 hunks)app/i18n/locales/pt-BR.json(1 hunks)app/i18n/locales/pt-PT.json(2 hunks)app/i18n/locales/ru.json(2 hunks)app/i18n/locales/sl-SI.json(1 hunks)app/i18n/locales/ta-IN.json(2 hunks)app/i18n/locales/te-IN.json(2 hunks)app/i18n/locales/tr.json(2 hunks)app/i18n/locales/zh-TW.json(1 hunks)app/views/RoomsListView/components/ServersList.tsx(4 hunks)app/views/RoomsListView/styles.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (16)
- app/i18n/locales/pt-PT.json
- app/i18n/locales/hi-IN.json
- app/i18n/locales/ru.json
- app/i18n/locales/ja.json
- app/i18n/locales/zh-TW.json
- app/i18n/locales/ta-IN.json
- app/i18n/locales/ar.json
- app/i18n/locales/de.json
- app/i18n/locales/pt-BR.json
- app/containers/ServerItem/Touchable.tsx
- app/containers/SwipeableDeleteItem/Actions.tsx
- app/i18n/locales/te-IN.json
- app/i18n/locales/bn-IN.json
- app/i18n/locales/hu.json
- app/i18n/locales/nn.json
- app/i18n/locales/tr.json
🧰 Additional context used
🧬 Code graph analysis (1)
app/views/RoomsListView/components/ServersList.tsx (1)
app/lib/constants/colors.ts (1)
colors(280-302)
🔇 Additional comments (10)
app/i18n/locales/nl.json (2)
15-16: New accessibility localization keys look good.The translations for server selection accessibility are contextually appropriate and correctly convey the intended meaning ("Activeren om server te selecteren" and the available delete action).
747-747: Dutch plural correction: "Werkruimten" → "Werkruimtes".This correction to the more modern Dutch plural form is appropriate. Both are technically valid, but "Werkruimtes" is the current standard.
app/i18n/locales/cs.json (1)
21-22: New a11y keys present and properly formatted.Both "Activate_to_select_server" and "Activate_to_select_server_Available_actions_delete" are correctly placed in alphabetical order with appropriate Czech translations. JSON formatting is valid.
app/i18n/locales/sl-SI.json (1)
19-20: Slovene locale now includes a11y keys; broader gap remains.Slovene (sl-SI) was one of 47 locales identified as missing "Activate_to_select_server" and "Activate_to_select_server_Available_actions_delete" in the prior review. This PR adds both keys with appropriate Slovenian translations.
However, 46 other locales (af, az, bas-CM, be-BY, bg, bn-BD, bn-IN, bs, ca, cy, da, de-AT, el, eo, et, eu, fa, gl, he, hi-IN, hr, id, ka-GE, kg, km, ko, ku, ln, lo, lt, lv, mn, ms-MY, nn, pl, ro, si, sk-SK, sq, sr, ta-IN, te-IN, th-TH, ug, uk, vi-VN, zh-HK) remain incomplete.
Verify that this PR is scoped to add these keys only to a subset of locales, or confirm whether the remaining 46 locales are also being updated in this changeset.
app/views/RoomsListView/styles.ts (2)
38-49: LGTM!The styling refinements improve text alignment and button appearance. The lineHeight adjustment for
serverHeaderAddand the added borderRadius forbuttonCreateWorkspacealign well with the layout refresh objectives.
50-52: LGTM!The new container style provides consistent spacing around the Add Server button, matching the updated layout design.
app/views/RoomsListView/components/ServersList.tsx (4)
1-26: LGTM!The import cleanup reflects the removal of the
createWorkspacefunction. Using named imports for log helpers improves code clarity.
123-133: Code changes verified and approved.The i18n key
Workspacesis properly defined across all 25+ supported locales with appropriate translations. The background color change fromsurfaceRoomtosurfaceLightand header text update toWorkspacesare correctly implemented and localized.
142-154: The i18n key 'Add_Server' is properly defined across all supported locales.Verification confirms the key exists in 23+ locale files (en, zh-TW, ar, zh-CN, tr, sl-SI, sv, te-IN, pt-PT, pt-BR, no, ru, nl, ja, hu, fr, es, hi-IN, de, fi, cs, bn-IN, it, ta-IN, and others). The button layout and styling are clean and consistent with the design system.
114-121: TheonDeletePressprop is correctly supported on the ServerItem component.The prop is defined in the
IServerIteminterface atapp/containers/ServerItem/index.tsx(line 23) as an optional callback, with proper fallback handling when not provided. The component implementation correctly passes this prop down to the underlyingTouchablecomponent, which integrates with the swipe-to-delete pattern. The usage in ServersList.tsx is correct.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
app/containers/SwipeableDeleteItem/Actions.tsx (1)
29-30: Consider documenting or making the padding constant configurable.The
SERVER_ITEM_PADDING_VERTICALconstant is hardcoded without explanation. If this value needs to match padding in related components (e.g., ServerItem), consider exporting it from a shared constants file or documenting why this specific value is used.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
app/containers/SwipeableDeleteItem/Actions.tsx(1 hunks)app/containers/SwipeableDeleteItem/Touchable.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
app/containers/SwipeableDeleteItem/Actions.tsx (1)
app/theme.tsx (1)
useTheme(29-29)
app/containers/SwipeableDeleteItem/Touchable.tsx (2)
app/theme.tsx (1)
useTheme(29-29)app/containers/SwipeableDeleteItem/Actions.tsx (1)
DeleteAction(31-126)
🔇 Additional comments (10)
app/containers/SwipeableDeleteItem/Actions.tsx (4)
1-27: LGTM!The imports and interface definition are clean and complete. All necessary types are properly imported.
42-57: LGTM!The RTL/LTR threshold detection logic is correct, and the haptic feedback timing is appropriate. The previousTransX comparison ensures the animation triggers only on threshold crossings.
59-98: LGTM!The RTL/LTR positioning logic correctly places the delete button on opposite sides and handles parallax animation for extended swipes. The dynamic left/right positioning ensures proper layout in both modes.
99-148: LGTM!The render structure correctly layers the delete button behind the content, and
pointerEvents='box-none'properly allows interaction with the foreground content. Accessibility is well-handled with labels and testID.app/containers/SwipeableDeleteItem/Touchable.tsx (6)
1-29: LGTM!The interface and imports are clean and complete. All necessary props for layout, interaction, and accessibility are defined.
31-83: LGTM!The component setup and handler functions correctly manage the swipe state lifecycle. The accessibility action support is properly implemented.
85-149: LGTM! RTL state machine logic is correct.The state machine properly handles both closed and open states with correct RTL/LTR directional logic. The previous concerns about redundant RTL branches have been addressed—RTL and LTR use opposite sign conventions (actionWidth vs -actionWidth) as expected.
151-178: LGTM! RTL gesture handling is now correct.The
onUpdatehandler now properly accounts for RTL directionality:
- RTL: Allows positive translations (right swipes) and blocks negative
- LTR: Allows negative translations (left swipes) and blocks positive
This addresses the previous review concern that the handler was blocking positive
transXunconditionally. The current implementation correctly inverts the directional logic based onI18n.isRTL.
180-182: LGTM!The animated style correctly applies the translateX transform to the swipeable content.
184-214: LGTM!The component structure correctly layers the DeleteAction behind the swipeable Touch component. Accessibility is well-implemented with both labels and custom actions for screen readers.
app/views/NewServerView/components/ServersHistoryItem/index.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
app/views/NewServerView/components/ServersHistoryItem/index.tsx (1)
44-46: Consider explicit handling for empty username.While React safely renders falsy values as empty, explicitly handling the empty case could improve clarity and allow for a placeholder if desired.
Optional refactor:
<Text numberOfLines={1} style={[styles.subtitle, { color: colors.fontSecondaryInfo }]}> - {item.username} + {item.username || ''} </Text>app/containers/ServerItem/SwipeableDeleteItem/Actions.tsx (1)
29-29: Consider extracting SERVER_ITEM_PADDING_VERTICAL to shared constants.The
SERVER_ITEM_PADDING_VERTICALconstant (value: 12) appears specific to server item layout. If this value is used elsewhere or needs to stay synchronized with padding defined instyles.ts, consider extracting it to a shared constants file to avoid duplication.#!/bin/bash # Check if this constant is duplicated or should be shared rg -n "SERVER_ITEM_PADDING|padding.*12" app/containers/ServerItem/ --type=ts --type=tsx
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (2)
app/containers/ServerItem/__snapshots__/ServerItem.test.tsx.snapis excluded by!**/*.snapapp/views/NewServerView/components/ServersHistoryItem/__snapshots__/ServersHistoryItem.test.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (30)
app/containers/ServerItem/SwipeableDeleteItem/Actions.tsx(1 hunks)app/containers/ServerItem/SwipeableDeleteItem/Touchable.tsx(1 hunks)app/containers/ServerItem/Touchable.tsx(1 hunks)app/containers/ServerItem/index.tsx(3 hunks)app/i18n/locales/ar.json(2 hunks)app/i18n/locales/bn-IN.json(2 hunks)app/i18n/locales/cs.json(1 hunks)app/i18n/locales/de.json(1 hunks)app/i18n/locales/en.json(2 hunks)app/i18n/locales/es.json(2 hunks)app/i18n/locales/fi.json(1 hunks)app/i18n/locales/fr.json(1 hunks)app/i18n/locales/hi-IN.json(2 hunks)app/i18n/locales/hu.json(1 hunks)app/i18n/locales/it.json(2 hunks)app/i18n/locales/ja.json(2 hunks)app/i18n/locales/nl.json(2 hunks)app/i18n/locales/nn.json(2 hunks)app/i18n/locales/no.json(2 hunks)app/i18n/locales/pt-BR.json(1 hunks)app/i18n/locales/pt-PT.json(2 hunks)app/i18n/locales/ru.json(2 hunks)app/i18n/locales/sl-SI.json(1 hunks)app/i18n/locales/sv.json(1 hunks)app/i18n/locales/ta-IN.json(2 hunks)app/i18n/locales/te-IN.json(2 hunks)app/i18n/locales/tr.json(2 hunks)app/i18n/locales/zh-CN.json(1 hunks)app/i18n/locales/zh-TW.json(1 hunks)app/views/NewServerView/components/ServersHistoryItem/index.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (20)
- app/i18n/locales/fr.json
- app/i18n/locales/hu.json
- app/i18n/locales/cs.json
- app/i18n/locales/de.json
- app/i18n/locales/ja.json
- app/i18n/locales/ru.json
- app/containers/ServerItem/Touchable.tsx
- app/i18n/locales/ta-IN.json
- app/i18n/locales/hi-IN.json
- app/i18n/locales/no.json
- app/i18n/locales/fi.json
- app/i18n/locales/te-IN.json
- app/i18n/locales/bn-IN.json
- app/i18n/locales/sv.json
- app/i18n/locales/tr.json
- app/i18n/locales/en.json
- app/i18n/locales/pt-PT.json
- app/i18n/locales/it.json
- app/i18n/locales/zh-CN.json
- app/i18n/locales/es.json
🧰 Additional context used
🧬 Code graph analysis (4)
app/containers/ServerItem/SwipeableDeleteItem/Actions.tsx (3)
app/theme.tsx (1)
useTheme(29-29)app/lib/constants/colors.ts (1)
colors(280-302)app/containers/CustomIcon/index.tsx (1)
CustomIcon(38-38)
app/containers/ServerItem/index.tsx (2)
app/theme.tsx (1)
useTheme(29-29)app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
useResponsiveLayout(41-45)
app/containers/ServerItem/SwipeableDeleteItem/Touchable.tsx (3)
app/theme.tsx (1)
useTheme(29-29)app/containers/ServerItem/SwipeableDeleteItem/Actions.tsx (1)
DeleteAction(31-126)app/lib/constants/colors.ts (1)
colors(280-302)
app/views/NewServerView/components/ServersHistoryItem/index.tsx (4)
app/definitions/IServerHistory.ts (1)
TServerHistoryModel(11-11)app/theme.tsx (1)
useTheme(29-29)app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (1)
useResponsiveLayout(41-45)app/lib/constants/colors.ts (1)
colors(280-302)
🔇 Additional comments (16)
app/i18n/locales/ar.json (1)
15-16: ✓ Translations properly placed and accurately rendered.The three new keys are correctly positioned in alphabetical order and use natural Arabic terminology:
- "Activate_to_select_server" and its compound accessibility variant align with the new swipe-to-delete server item gestures
- "Workspaces" follows standard terminology
JSON structure is valid. Translations are appropriately scoped for the server/workspace management UI updates described in the PR objectives.
Also applies to: 609-609
app/i18n/locales/nn.json (1)
14-14: Localization additions are well-structured and alphabetically ordered.The new keys are placed correctly in alphabetical sequence, translations are grammatically sound Norwegian Nynorsk, and the additions align with the PR objective to enhance Servers History/List UI. The three new entries support the swipe-to-delete interaction and workspace selection features.
Also applies to: 394-394
app/views/NewServerView/components/ServersHistoryItem/index.tsx (4)
22-25: Good use of responsive hook.The
useResponsiveLayout()hook at line 24 provides reactive width updates, addressing the previous review feedback about making the component responsive.
26-28: Excellent accessibility implementation.The conditional accessibility label and localized hint provide good screen reader support across locales.
1-53: Well-structured component with strong fundamentals.The component demonstrates good practices:
- Performance optimization via
React.memo- Comprehensive accessibility (label, hint)
- Theme-aware styling with dynamic colors
- Responsive layout via
useResponsiveLayout- Clean TypeScript interface with public exports
- Proper image handling with fallback
38-38: Database schema properly includes iconURL support.Verification confirms that
iconURLis correctly defined throughout the stack:
- Database Schema (
app/lib/database/schema/servers.js):icon_urlcolumn defined as optional string- Migration (
app/lib/database/model/servers/migrations.js, version 17): Addsicon_urlcolumn toservers_historytable- WatermelonDB Model (
app/lib/database/model/ServersHistory.js, line 15):@field('icon_url') iconURLproperly maps the database column- TypeScript Interface (
app/definitions/IServerHistory.ts, line 8):iconURL?: stringmatches the schemaThe code at line 38 safely uses
item.iconURLwith a fallback todefaultLogo, and the optional property definition allows for backward compatibility.app/i18n/locales/pt-BR.json (1)
21-22: LGTM! New accessibility strings are clear and appropriate.The two new translation keys provide clear accessibility labels for server selection and the delete action, properly localized for pt-BR.
app/i18n/locales/nl.json (2)
15-16: LGTM! Dutch accessibility strings are clear and consistent.The two new translation keys match the structure and intent of the pt-BR translations, properly localized for Dutch.
749-749: Workspace terminology in nl.json is correctly and consistently applied.All workspace-related translations in the file use "werkruimte" (singular) where appropriate, and line 749 correctly uses "Werkruimtes" as the plural form for the "Workspaces" key. There are no inconsistencies or terminology conflicts.
app/containers/ServerItem/SwipeableDeleteItem/Touchable.tsx (2)
151-178: LGTM! Pan gesture configuration with RTL-aware directional constraints.The gesture handling properly accounts for RTL layouts:
- RTL: Allows positive translations (right swipes), blocks negative
- LTR: Allows negative translations (left swipes), blocks positive
The
activeOffsetXandfailOffsetYconfiguration appropriately distinguishes horizontal swipes from vertical scrolling.
85-149: State machine transitions are correctly implemented with proper RTL/LTR symmetry and edge case handling.The
handleReleasestate machine has been verified for all edge cases:
State 0 → 1 (open delete): The
longSwipethreshold correctly triggers immediate delete in both RTL (translationX >= longSwipe) and LTR (translationX <= -longSwipe), withhandleDeletePress()resetting state immediately.State 1 → 0 (close): RTL and LTR thresholds are symmetric—RTL closes when
valueRef.current < smallSwipewhile LTR closes whenvalueRef.current > -smallSwipe, both properly balanced around zero.Long swipe in state 1: No conflicts exist. When triggered from State 0,
handleDeletePress()callsclose()(lines 70-75) which resetsrowStateto 0 andvalueRef.currentto 0, preventing the code from remaining in State 1 with accumulated values that could re-trigger delete logic.The gesture handler's
onUpdatemethod (lines 154-175) also includes directional constraints that prevent swipes opposite to the intended direction, reinforcing the RTL/LTR correctness.app/containers/ServerItem/index.tsx (2)
32-32: LGTM! Responsive width calculation properly implemented.Using
useResponsiveLayout()ensures the width recalculates on orientation changes and tablet window resizes, addressing the past review concern about static width values.
34-38: LGTM! Accessibility labels and hints are well-implemented.The accessibility strings properly combine server name and ID, with conditional hints that clearly communicate available actions to screen reader users.
app/containers/ServerItem/SwipeableDeleteItem/Actions.tsx (3)
42-57: LGTM! Well-implemented haptic feedback on threshold crossing.The
useAnimatedReactioncorrectly detects when the swipe crosses thelongSwipethreshold in both directions and triggers haptic feedback with a smooth spring animation. The RTL/LTR branching properly handles directional logic.
59-98: LGTM! Parallax effect adds visual polish.The animated styles implement a nice parallax effect for long swipes (beyond
2 * actionWidth), with proper RTL/LTR positioning. The interpolation creates smooth visual feedback as the user continues swiping.
114-121: LGTM! Delete button is properly accessible.The
RectButtonincludes an appropriate accessibility label usingI18n.t('Delete'), making the action discoverable to screen reader users.
| }, | ||
| { | ||
| "width": 750, | ||
| "width": undefined, |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/views/RoomsListView/components/ServersList.tsx (1)
114-121: Confusing and potentially incorrect deletion logic.The expression
item.id === server || remove(item.id)misuses the||operator for conditional execution. While it may technically prevent deletion of the current server via short-circuit evaluation, the logic is non-idiomatic and unclear. Additionally, the past review comment suggests this approach "doesn't work properly."Apply this diff to use explicit conditional logic:
- const renderItem = ({ item }: { item: { id: string; iconURL: string; name: string; version: string } }) => ( - <ServerItem - item={item} - onPress={() => select(item.id, item.version)} - onDeletePress={() => item.id === server || remove(item.id)} - hasCheck={item.id === server} - /> - ); + const renderItem = ({ item }: { item: { id: string; iconURL: string; name: string; version: string } }) => ( + <ServerItem + item={item} + onPress={() => select(item.id, item.version)} + onDeletePress={() => { + if (item.id !== server) { + remove(item.id); + } + }} + hasCheck={item.id === server} + /> + );Alternatively, if
ServerItemsupportsundefinedforonDeletePressto disable deletion:- onDeletePress={() => item.id === server || remove(item.id)} + onDeletePress={item.id !== server ? () => remove(item.id) : undefined}
♻️ Duplicate comments (1)
app/i18n/locales/sl-SI.json (1)
19-20: Align Slovenian grammar across related accessibility keys (unresolved from prior review).The previous review flagged a grammatical inconsistency between lines 19 and 20. Line 19 uses the accusative form "izbiro" while line 20 uses the nominative form "izbor," creating inconsistency. The English source repeats the same phrase in both keys, and other locales (e.g., Spanish) maintain consistent phrasing. Update line 20's value to match line 19's grammatical form:
- "Activate_to_select_server_Available_actions_delete": "Aktiviraj za izbor strežnika. Na voljo dejanja: izbriši", + "Activate_to_select_server_Available_actions_delete": "Aktiviraj za izbiro strežnika. Na voljo dejanja: izbriši",
🧹 Nitpick comments (2)
app/views/NewServerView/components/ServersHistoryItem/ServersHistoryItem.stories.tsx (2)
17-23: Use a fixedupdatedAtvalue to keep stories/tests deterministicUsing
new Date()on Line 21 makes the story time-dependent; ifupdatedAtis rendered, snapshots or visual comparisons may change over time. Prefer a fixed example timestamp instead.- updatedAt: new Date(), + updatedAt: new Date('2024-01-01T00:00:00.000Z'),
25-33: Consider adding variants that exercise condensed and large-font layouts
responsiveLayoutProviderValuecurrently always usesBASE_ROW_HEIGHT,BASE_ROW_HEIGHT_CONDENSED, andfontScale: 1(Lines 25–32), so all stories (Lines 61–88) render only the default layout path. You could add additional stories that overrideisLargeFontScale,fontScale, orrowHeight/rowHeightCondensedto visually validate condensed rows and accessibility/large-font behavior.Also applies to: 61-88
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (2)
app/containers/ServerItem/__snapshots__/ServerItem.test.tsx.snapis excluded by!**/*.snapapp/views/NewServerView/components/ServersHistoryItem/__snapshots__/ServersHistoryItem.test.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (28)
app/i18n/locales/ar.json(2 hunks)app/i18n/locales/bn-IN.json(2 hunks)app/i18n/locales/cs.json(1 hunks)app/i18n/locales/de.json(1 hunks)app/i18n/locales/en.json(2 hunks)app/i18n/locales/es.json(2 hunks)app/i18n/locales/fi.json(1 hunks)app/i18n/locales/fr.json(1 hunks)app/i18n/locales/hi-IN.json(2 hunks)app/i18n/locales/hu.json(1 hunks)app/i18n/locales/it.json(2 hunks)app/i18n/locales/ja.json(2 hunks)app/i18n/locales/nl.json(2 hunks)app/i18n/locales/nn.json(2 hunks)app/i18n/locales/no.json(2 hunks)app/i18n/locales/pt-BR.json(1 hunks)app/i18n/locales/pt-PT.json(2 hunks)app/i18n/locales/ru.json(2 hunks)app/i18n/locales/sl-SI.json(1 hunks)app/i18n/locales/sv.json(1 hunks)app/i18n/locales/ta-IN.json(2 hunks)app/i18n/locales/te-IN.json(2 hunks)app/i18n/locales/tr.json(2 hunks)app/i18n/locales/zh-CN.json(1 hunks)app/i18n/locales/zh-TW.json(1 hunks)app/views/NewServerView/components/ServersHistoryItem/ServersHistoryItem.stories.tsx(1 hunks)app/views/NewServerView/components/ServersHistoryItem/ServersHistoryItem.test.tsx(1 hunks)app/views/RoomsListView/components/ServersList.tsx(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (22)
- app/i18n/locales/pt-BR.json
- app/i18n/locales/hi-IN.json
- app/i18n/locales/cs.json
- app/i18n/locales/de.json
- app/i18n/locales/zh-CN.json
- app/i18n/locales/ru.json
- app/i18n/locales/bn-IN.json
- app/views/NewServerView/components/ServersHistoryItem/ServersHistoryItem.test.tsx
- app/i18n/locales/nl.json
- app/i18n/locales/es.json
- app/i18n/locales/ar.json
- app/i18n/locales/ta-IN.json
- app/i18n/locales/no.json
- app/i18n/locales/fi.json
- app/i18n/locales/nn.json
- app/i18n/locales/zh-TW.json
- app/i18n/locales/en.json
- app/i18n/locales/te-IN.json
- app/i18n/locales/sv.json
- app/i18n/locales/fr.json
- app/i18n/locales/hu.json
- app/i18n/locales/it.json
🧰 Additional context used
🧬 Code graph analysis (2)
app/views/NewServerView/components/ServersHistoryItem/ServersHistoryItem.stories.tsx (5)
app/definitions/IServerHistory.ts (1)
TServerHistoryModel(11-11)app/lib/hooks/useResponsiveLayout/useResponsiveLayout.tsx (3)
BASE_ROW_HEIGHT(21-21)BASE_ROW_HEIGHT_CONDENSED(22-22)ResponsiveLayoutContext(18-18)app/theme.tsx (2)
TSupportedThemes(8-8)ThemeContext(18-18)app/views/NewServerView/components/ServersHistoryItem/index.tsx (1)
IServersHistoryItem(14-18)app/lib/constants/colors.ts (1)
themes(304-304)
app/views/RoomsListView/components/ServersList.tsx (2)
app/lib/constants/colors.ts (1)
colors(280-302)app/containers/message/Components/Attachments/Image/Button.tsx (1)
Button(13-26)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: ESLint and Test / run-eslint-and-test
- GitHub Check: format
🔇 Additional comments (6)
app/i18n/locales/tr.json (2)
15-16: Accessibility localization keys added correctly.The new Turkish translations for
Activate_to_select_serverandActivate_to_select_server_Available_actions_deleteare properly formatted and align with the swipe-to-delete feature context. The phrasing is natural and accessible in Turkish.
640-640: Workspaces capitalization aligned with other locales.The update from lowercase to capitalized "Çalışma Alanları" improves consistency across the i18n locales and matches the UI pattern used elsewhere in the codebase.
app/i18n/locales/pt-PT.json (1)
15-16: Localization additions in pt-PT.json are correct, but incomplete translation coverage exists across supported locales.The three new keys (
Activate_to_select_server,Activate_to_select_server_Available_actions_delete,Workspaces) are properly added to pt-PT.json with accurate Portuguese translations and correct alphabetical ordering.However, verification shows these keys are only present in 25 out of 67 supported locales (37% coverage). The application uses
i18n.fallbacks = truewith English (en.json) as the default fallback locale, so incomplete translations will gracefully fall back to English. While this is expected and acceptable, it should be noted that for a production-quality user experience, translations for at least the major locales (ar, de, es, fr, pt-BR, ru, zh-CN, etc.) should be prioritized.The review comment's verification suggestion was sound and confirmed the fallback mechanism is in place to handle missing translations in less-frequently-used locales.
app/views/NewServerView/components/ServersHistoryItem/ServersHistoryItem.stories.tsx (1)
35-59: Wrapper component composition and typing look solidThe
ServersHistoryItemstory wrapper correctly wiresThemeContextandResponsiveLayoutContextand acceptsPartial<TServerHistoryModel>so stories can override only the needed fields while keeping sensible defaults. TheonPress/onDeletePresstypings viaIServersHistoryItemkeep the story API aligned with the production component.app/views/RoomsListView/components/ServersList.tsx (2)
124-133: LGTM! UI improvements align with PR objectives.The background color change to
surfaceLightand label update to "Workspaces" are consistent with the new action sheet layout design described in the PR objectives.
143-153: All style and color token definitions are properly defined and exist.Verification confirms:
styles.addServerButtonContainer(line 50, RoomsListView/styles.ts)styles.buttonCreateWorkspace(line 43, RoomsListView/styles.ts)colors.buttonFontSecondary(line 86, lib/constants/colors.ts)colors.buttonBackgroundSecondaryDefault(line 68, lib/constants/colors.ts)The style name
buttonCreateWorkspacedoes not need to match the button text "Add_Server"—style reuse across different components is standard practice and poses no risk. No runtime errors will occur.
2 participants
Proposed changes
Improve Servers History and Servers List action sheet layout.
Issue(s)
https://rocketchat.atlassian.net/browse/NATIVE-1063
How to test or reproduce
Open the app.
Log in to two servers.
Tap the Servers List button.
Swipe on a workspace and tap Delete.
After logging in to two workspaces, remove one of them.
Tap the Servers List button.
Tap Add Workspace.
Tap the Servers History button.
Swipe to delete the workspace that was previously removed.
Screenshots
Servers List
Servers History
RTL
Types of changes
Checklist
Further comments
Summary by CodeRabbit
New Features
Improvements
Localization
✏️ Tip: You can customize this high-level summary in your review settings.