TT-6814-17, TT-6428 Add Phrase Back Translation mobile layout#195
TT-6814-17, TT-6428 Add Phrase Back Translation mobile layout#195
Conversation
gtryus
commented
Jan 30, 2026
gtryus
commented
- Introduced Cypress component testing takeaways to guide best practices in testing setups and assertions.
- Added Jest testing takeaways for unit tests, emphasizing mocking strategies and edge case coverage.
- Refactored MediaRecord, WSAudioPlayer, and related components to improve props handling and mobile responsiveness.
- Updated UserMenu and PlanTabs components to utilize new mobile utility functions for better layout management.
There was a problem hiding this comment.
Pull request overview
This PR introduces a mobile-focused UI flow (especially for Phrase Back Translation) and consolidates “mobile mode” detection/behavior across the app, while extending audio player/recording controls and adding supporting localization + tests.
Changes:
- Added a shared
useMobile()hook and refactored multiple screens/components to use it for responsive/layout decisions. - Implemented new Passage Detail mobile layout components (header/footer/segment controls, navigation) and updated audio player/recording components to support externally-driven controls and mobile UX.
- Extended localization keys/selectors and added Jest + Cypress component tests for new utilities and mobile components.
Reviewed changes
Copilot reviewed 56 out of 58 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| src/renderer/src/utils/useMobile.ts | New hook to unify mobile-width + user “mobile view” toggle. |
| src/renderer/src/utils/useMobile.test.ts | Jest coverage for useMobile sync and derived flags. |
| src/renderer/src/utils/index.ts | Exports useMobile. |
| src/renderer/src/store/localization/reducers.tsx | Adds mobile, recordButton layouts + reset string. |
| src/renderer/src/store/localization/model.tsx | Adds typings for mobile, recordButton, and wsAudioPlayer.reset. |
| src/renderer/src/store/localization/exported-strings-name.json | Updates exported strings bundle name. |
| src/renderer/src/selector/selectors.tsx | Adds mobileSelector and recordButtonSelector. |
| src/renderer/src/routes/TeamScreen.tsx | Uses useMobile() for layout branching. |
| src/renderer/src/routes/SwitchTeams.tsx | Uses useMobile() for responsive styling. |
| src/renderer/src/routes/ProjectsScreen.tsx | Uses useMobile() for responsive layout decisions. |
| src/renderer/src/routes/PassageDetail.tsx | Mobile-specific layout behavior for Phrase BT and effect dependency changes. |
| src/renderer/src/main.tsx | Initializes mobileView from persisted local storage key. |
| src/renderer/src/crud/prevPasId.ts | Adds previous-passage navigation helper. |
| src/renderer/src/crud/prevPasId.test.ts | Jest tests for prevPasId edge cases. |
| src/renderer/src/control/smallBtnProps.ts | Adds shared small button styling props. |
| src/renderer/src/control/index.ts | Exports smallBtnProps. |
| src/renderer/src/control/RecordButton.tsx | Extends RecordButton to support text mode, stop logic, styling overrides, localization. |
| src/renderer/src/control/RecordButton.cy.tsx | Cypress CT coverage for RecordButton variants. |
| src/renderer/src/context/UnsavedContext.tsx | Adds forceClearPending to clear unsaved state on confirm flows. |
| src/renderer/src/context/PassageDetailContext.tsx | Adjusts step setting call to use the internal handler. |
| src/renderer/src/components/WSAudioPlayerSegment.tsx | Adds mobile/dev gating for settings/delete + alternate remove icon option. |
| src/renderer/src/components/WSAudioPlayer.tsx | Adds external controls ref + hide flags; mobile toolbar/menu adjustments. |
| src/renderer/src/components/UserMenu.tsx | Uses useMobile() and simplifies mobile toggle sync logic. |
| src/renderer/src/components/Team/ProjectMenu.tsx | Uses useMobile() for responsive behavior. |
| src/renderer/src/components/Sheet/ScriptureTable.tsx | Uses useMobile() and simplifies mobile layout switch. |
| src/renderer/src/components/Sheet/PassageCard.tsx | Uses useMobile() for card width behavior. |
| src/renderer/src/components/PlanTabs.tsx | Uses useMobile() to switch section/passage tab behavior on mobile. |
| src/renderer/src/components/PassageDetail/mobile/WorkflowStepsMobile.tsx | Wrapper for workflow steps in mobile layout. |
| src/renderer/src/components/PassageDetail/mobile/WorkflowStepsMobile.cy.tsx | Cypress CT coverage for workflow steps wrapper interactions. |
| src/renderer/src/components/PassageDetail/mobile/SegmentStatusMobile.tsx | Mobile segment status display using mobile strings. |
| src/renderer/src/components/PassageDetail/mobile/SegmentStatusMobile.cy.tsx | Cypress CT coverage for segment status behavior. |
| src/renderer/src/components/PassageDetail/mobile/SegmentControlsMobile.tsx | Mobile segment control buttons with localized tooltips. |
| src/renderer/src/components/PassageDetail/mobile/PassageDetailMobileLayout.tsx | New mobile page shell (header/content/footer). |
| src/renderer/src/components/PassageDetail/mobile/PassageDetailMobileFooter.tsx | Mobile footer with prev/next navigation + step complete. |
| src/renderer/src/components/PassageDetail/mobile/PassageDetailMobileFooter.cy.tsx | Cypress CT coverage for footer navigation + persistence. |
| src/renderer/src/components/PassageDetail/mobile/PassageDetailMobileContext.tsx | Mobile header context (reference + project name). |
| src/renderer/src/components/PassageDetail/mobile/PassageDetailMobileContext.cy.tsx | Cypress CT coverage for context rendering. |
| src/renderer/src/components/PassageDetail/mobile/PassageAudioHeaderMobile.tsx | Mobile audio header controls for play + segment nav. |
| src/renderer/src/components/PassageDetail/mobile/PassageAudioHeaderMobile.cy.tsx | Cypress CT coverage for audio header interactions/states. |
| src/renderer/src/components/PassageDetail/mobile/MobileWorkflowSteps.tsx | Mobile workflow step chooser UI. |
| src/renderer/src/components/PassageDetail/mobile/MobileWorkflowSteps.cy.tsx | Cypress CT coverage for mobile workflow steps behavior. |
| src/renderer/src/components/PassageDetail/mobile/MobileStepComplete.tsx | Mobile step completion control with permission gating. |
| src/renderer/src/components/PassageDetail/mobile/MobileStepComplete.cy.tsx | Cypress CT coverage for mobile step completion logic. |
| src/renderer/src/components/PassageDetail/PassageDetailPlayer.tsx | Adds mobile height, external controls, and improved “segments changed” behavior. |
| src/renderer/src/components/PassageDetail/PassageDetailItem.tsx | Introduces Phrase BT mobile layout composition and recorder/player control wiring. |
| src/renderer/src/components/MediaRecord.tsx | Adds external controls + hide flags; adjusts wrapper/size rendering behavior. |
| src/renderer/src/components/Discussions/DiscussionPanel.tsx | Adjusts panel sizing to window width and offsets FAB on detail routes. |
| src/renderer/src/components/Discussions/DiscussionList.tsx | Removes stale hook-deps suppression comments. |
| src/renderer/src/components/App/OrgHead.tsx | Uses useMobile() for responsive behavior. |
| src/renderer/src/components/App/MobileDetailTitle.tsx | Adds compact title for mobile detail header. |
| src/renderer/src/components/App/MobileDetailTitle.cy.tsx | Cypress CT coverage for mobile detail title resolution/rendering. |
| src/renderer/src/components/App/AppHead.tsx | Uses useMobile() and swaps OrgHead with MobileDetailTitle on detail routes; safe back nav. |
| localization/TranscriberAdmin-en.xlf | Adds new string units for mobile + record button + reset. |
| localization/TranscriberAdmin-en-1.2.xliff | Adds new string units for mobile + record button + reset (xliff). |
| .cursor/rules/jest-testing-takeaways.mdc | Adds repo-specific Jest testing guidance. |
| .cursor/rules/cypress-testing-takeaways.mdc | Adds repo-specific Cypress CT testing guidance. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| !canSave || | ||
| (ArtifactTypeSlug.PhraseBackTranslation === recordType && | ||
| (segString || '{}') === '{}') |
There was a problem hiding this comment.
This negation always evaluates to false.
| !canSave || | |
| (ArtifactTypeSlug.PhraseBackTranslation === recordType && | |
| (segString || '{}') === '{}') | |
| ArtifactTypeSlug.PhraseBackTranslation === recordType && | |
| (segString || '{}') === '{}' |
There was a problem hiding this comment.
This is not a correct fix either.
| return ( | ||
| <Button | ||
| variant="outlined" | ||
| color={recording ? 'error' : 'primary'} |
There was a problem hiding this comment.
This use of variable 'recording' always evaluates to false.
There was a problem hiding this comment.
I don't this this is correct??
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 60 out of 62 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| useEffect(() => { | ||
| setAdding(false); | ||
| // eslint-disable-next-line react-hooks/exhaustive-deps | ||
| }, [currentstep, passage]); |
There was a problem hiding this comment.
Disabled eslint comment was removed from dependency array. Verify that all dependencies used within the effect are included in the dependency array to prevent stale closures.
| <IconButton | ||
| onClick={() => checkSavedFn(() => navigate(planUrl || '/team'))} | ||
| > |
There was a problem hiding this comment.
Creating a new function on every render can cause unnecessary re-renders. Consider wrapping the navigation callback with useCallback or extracting it to a named handler function.
gtryus
force-pushed
the
feature/TT-6814-detail-context
branch
from
640389f to
1bd90d4
Compare
gtryus
force-pushed
the
feature/TT-6814-detail-context
branch
2 times, most recently
from
7d7931c to
cd528a7
Compare
|
I've had a quick look through the page (on Electron), and though it's getting there, I was able to find a number of issues. I'm not sure how much of this we need to fix now, but I'd think everything should get fixed before release. I didn't delve into the code at this point. I also tried not to be too picky, and hopefully these comments don't seem so.
|
gtryus
force-pushed
the
feature/TT-6814-detail-context
branch
from
e25a3ea to
ec197dd
Compare
- Introduced Cypress component testing takeaways to guide best practices in testing setups and assertions. - Added Jest testing takeaways for unit tests, emphasizing mocking strategies and edge case coverage. - Refactored MediaRecord, WSAudioPlayer, and related components to improve props handling and mobile responsiveness. - Updated UserMenu and PlanTabs components to utilize new mobile utility functions for better layout management. - Introduced Cypress component testing takeaways to guide best practices in testing setups and assertions. - Added Jest testing takeaways for unit tests, emphasizing mocking strategies and edge case coverage. - Refactored MediaRecord, WSAudioPlayer, and related components to improve props handling and mobile responsiveness. - Updated UserMenu and PlanTabs components to utilize new mobile utility functions for better layout management. - feedback fixes - Highlight and play segment logic - convert in line to functions - add test for HighlightButton - updates for preview and auto save - move steps to header - play entire recording preview - auto segment highlight step - remove scroll bar from steps - remove navigation player navigation buttons - remove previous segment control - delete leads to record - auto save segments - auto save recordings - click in segment starts play at beginning - update logic for working through the steps - only show undo if possible - dont show clear all segments - show play and pause for player - add undo for segments - first click positions at start of the region. - second click in a region positions on click
gtryus
force-pushed
the
feature/TT-6814-detail-context
branch
from
ec197dd to
1fc57f9
Compare
ResponsesI've had a quick look through the page (on Electron), and though it's getting there, I was able to find a number of issues. I'm not sure how much of this we need to fix now, but I'd think everything should get fixed before release. I didn't delve into the code at this point. I also tried not to be too picky, and hopefully these comments don't seem so. I think a theme/direction is important here, and it seems to be missing. This is pretty difficult to get right, and maybe even difficult to see, but the first thing I notice on this page is that everything is different. There's little uniformity in the buttons, for example. ☐ You've got a big blue button and small blue button,
☑ you've got a blue button with white text and then a white button with blue text,
☑ you've got a giant red circle or a disabled button with text in all caps, some gray-ish icon buttons, and a black floating button that overlaps its background. I'm not sure if this is because it's from AI, or it's in an intermediate state, but I hope we have plans to improve the overall look before release. I think we would benefit from working on the layout and theme styling, but maybe that's meant to come in a separate task. ☑ I wouldn't have Play and duration (etc.) anchored to the top. I think it's a little confusing as it is, and I don't think it's very valuable with how little, if any, scrolling there is on this page. ☐ When there's a scrollbar, there's also a forced horizontal scrollbar. This is I think because we're hardcoding/calculating the audio player to a fixed width and not recalculating that when the screen resizes vertically. It functions, but it looks odd.
☑ Similarly, note that the "centered" items aren't actually centered. If you look in the Dev Tools (or smaller screens) this is obvious.
☑ The "Segment" display label should be converted from its raw number values to M:SS, or MM:SS if applicable.
☑ Under the waveform, remove Select Segments, unless it was intentional. I'm not sure what that's doing. ☑ Can we hide the Undo icon on page load? If you look at the design, it does have a disabled state, but it's also not shown initially. ☑ We need a way to zoom the waveform. Though not pictured in many of the designs, my plan is for scrolling the mouse wheel (or pinch on mobile) to zoom. Any zoomed state would cause a horizontal scrollbar under the waveform to appear, for use when panning. Maybe this is a separate task?
☐ I also notice that the Plus/Minus buttons don't match the design. Separate task?
☑ When using the Next button, the segment starts playing, but the Play doesn't change. This makes it unclear how to Pause. ☑ Similarly, in some/all cases, the Play button doesn't change to Pause the first time you click it. It does on the second click. Or maybe I'm in some other bugged state? ☑ Clicking Play restarts the segment from the beginning, so there's no way to pause while playing the segment. ☑ Sometimes clicking Next appears to take you to the next segment, but the BT loaded is the one from the previous segment. Then, if you pause and play, the play marker pops back to the previous segment. I also see this when clicking in a segment. It seems bugged. ☑ The Record appears to record even when the button is disabled.
☑ Picky: the back translation Play button is not aligned with the main Play, and the hover effect is also messed up.
Other items to correct:☑ Walk through the basic flow for two segments ☑ Decide what the active v. highlighted button should look like ☑ After preview, clicking auto segment or + or - should enable listening ☑ Allow clicking elsewhere within current segment (don't always start at the beginning) ☑ Jump to beginning when clicking in any segment other than current segment ☐ Create cypress scenarios to test PBT with mobile ☑ Make sure segment save waits long enough ☐ Disable buttons while saving is taking place. |
2 participants
