TT-7017 handle flat projects with title on card#192
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request adds support for flat projects by introducing a new FlatTitle component that displays section titles for passages in flat project structures. The PR also includes comprehensive Cypress testing setup and documentation, along with cleanup of unused ESLint comments.
Changes:
- Created
FlatTitlecomponent to render section titles forSectionPassagekind passages - Updated
PlanViewto renderPassageCardfor bothIwsKind.PassageandIwsKind.SectionPassagekinds - Added comprehensive Cypress testing guide and test coverage for new components
- Removed unnecessary ESLint disable comments for react-hooks/exhaustive-deps
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/renderer/src/components/Sheet/FlatTitle.tsx | New component that displays section titles for flat project passages |
| src/renderer/src/components/Sheet/FlatTitle.cy.tsx | Comprehensive Cypress tests for the FlatTitle component |
| src/renderer/src/components/Sheet/PassageCard.tsx | Integrated FlatTitle component and cleaned up formatting |
| src/renderer/src/components/Sheet/PassageCard.cy.tsx | Added tests for FlatTitle integration in PassageCard |
| src/renderer/src/components/Sheet/PlanView.tsx | Updated to handle both Passage and SectionPassage kinds |
| src/renderer/src/components/Sheet/PlanView.cy.tsx | Added tests for rendering PassageCard with different kinds |
| src/renderer/src/components/Sheet/usePlanSheetFill.tsx | Removed unused ESLint comment |
| src/renderer/src/components/Sheet/ScriptureTable.tsx | Removed unused ESLint comments and reformatted code |
| src/renderer/src/components/Sheet/PlanSheet.tsx | Removed unused ESLint comment |
| .github/instructions/cypress-testing.instructions.md | New comprehensive guide for Cypress component testing patterns |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
9e122a6 to
9ab2331
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 15 changed files in this pull request and generated 6 comments.
Comments suppressed due to low confidence (4)
src/renderer/src/components/Sheet/PassageCard.cy.tsx:128
- The PassageCard component no longer accepts a 'getBookName' prop (removed in PassageCard.tsx line 22), but tests still reference it in the props interface and pass it to the component. This will cause test failures. Remove the getBookName prop from the mountPassageCard helper function's props parameter type and from line 186 where it's passed to the component. Update all test cases that reference mockGetBookName accordingly.
getBookName: (bookAbbreviation: string | undefined) => string;
src/renderer/src/components/Sheet/PassageCard.cy.tsx:195
- PassageCard now uses PassageRef which depends on PlanContext (PassageRef.tsx line 17), but the test mount doesn't provide a PlanContext.Provider. This will cause tests to fail. Add PlanContext.Provider with appropriate mock state to the provider stack, similar to how PlanView.cy.tsx does it (lines 168-189).
cy.mount(
<Provider store={mockStore}>
<GlobalProvider init={finalInitialState}>
<DataProvider dataStore={mockMemoryWithSections}>
<PassageCard
cardInfo={cardInfo}
getBookName={props.getBookName}
handleViewStep={props.handleViewStep}
onPlayStatus={props.onPlayStatus}
isPlaying={props.isPlaying}
isPersonal={props.isPersonal}
/>
</DataProvider>
</GlobalProvider>
</Provider>
);
src/renderer/src/components/Sheet/PassageCard.cy.tsx:63
- PassageRef component requires state.books.map for book name resolution (PassageRef.tsx line 19), but the mock books reducer returns an empty object. Add a proper book map to the reducer for tests to work correctly, similar to PassageRef.cy.tsx lines 54-61 which provides a map with book abbreviations.
const mockStore = createStore(
combineReducers({
strings: mockStringsReducer,
books: () => ({}),
src/renderer/src/components/Sheet/PassageCard.cy.tsx:303
- This test validates getBookName is called with the book abbreviation, but since PassageCard no longer receives or uses getBookName prop (it's now handled internally by PassageRef), this test is no longer valid. Either remove this test or update it to verify the book name resolution happens correctly through the Redux store and PlanContext.
it('should call getBookName with book abbreviation', () => {
const cardInfo = createMockSheet({ book: 'MAT' });
mountPassageCard(cardInfo, {
getBookName: mockGetBookName,
handleViewStep: mockHandleViewStep,
isPlaying: false,
});
cy.wrap(mockGetBookName).should('have.been.calledWith', 'MAT');
});
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
063a180 to
4ee8873
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
src/renderer/src/components/Sheet/PassageCard.cy.tsx:1
- Test is checking for the string 'undefined' in the output, which indicates the getBookName function is returning undefined as a string rather than handling missing book mappings correctly. This should return 'Unknown Book' or the abbreviation itself according to the PassageRef implementation logic.
import React from 'react';
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
4ee8873 to
047a411
Compare
- Introduced comprehensive Cypress testing instructions for component testing in the APM Vite application, detailing setup patterns, mocking strategies, and debugging tips. - Added a guide for writing Jest tests for React hooks, addressing common challenges and providing patterns for effective testing. - Updated PassageCard and related components to utilize new testing patterns and improved mock data structures. - Created new PassageGraphic and PassageRef components to enhance rendering logic and improve testability.
047a411 to
ce8f599
Compare
Uh oh!
There was an error while loading. Please reload this page.