PR Template, button bugfix and restructure 'navigator' component #43
Conversation
…their own directory. Improved code readability. Removed unused baseURL variable.
theartcher
added
bug
There was a problem hiding this comment.
Pull request overview
This PR restructures the navigator component by extracting models and interfaces into a dedicated models directory, adds a pull request template, updates README guidelines for clarity, and fixes the responsive button layout on mobile devices.
Key Changes:
- Extracted type definitions (YesOrNo, NodeType, DecisionNode, DecisionTree, Answer) into separate files under
src/models/ - Moved helper functions from
src/data/decision-tree.tsto newsrc/utils/decision-tree-helper.tsutility file - Fixed mobile button layout by adjusting CSS min-width from 100% to 30vw and removing vertical stacking
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
src/models/yes-or-no.ts |
New file defining the YesOrNo type (previously in QuestionCard.tsx) |
src/models/node-type.ts |
New file defining the NodeType type (previously in decision-tree.ts) |
src/models/decision-tree.ts |
New file defining the DecisionTree interface (previously in decision-tree.ts) |
src/models/decision-node.ts |
New file defining the DecisionNode interface (previously in decision-tree.ts) |
src/models/answer.ts |
New file defining the Answer interface (previously in NavigatorContainer.tsx) |
src/utils/decision-tree-helper.ts |
New utility file with helper functions moved from decision-tree.ts, but contains incorrect import paths |
src/data/decision-tree.ts |
Removed type definitions and helper functions, now only exports decisionTree data |
src/components/navigator/QuestionCard.tsx |
Updated to import YesOrNo from models directory instead of exporting it |
src/components/navigator/NavigatorContainer.tsx |
Updated imports to use new model and utility file locations |
src/components/navigator/AnsweredQuestion.tsx |
Updated imports and added logic to prevent redundant answer changes |
src/components/navigator/PatternResult.tsx |
Removed unused baseURL constant |
src/components/navigator/navigator.module.css |
Fixed button layout for mobile by adjusting min-width and removing flex-direction override |
README.md |
Clarified coding standards: corrected "Queens" to "Queen's", specified pattern pages for template, and clarified "source code" for TODO guideline |
.github/pull_request_template.md |
Added new PR template with sections for description, type of change, validation, checklist, and screenshots |
| @@ -0,0 +1,36 @@ | |||
| import { YesOrNo } from "../components/navigator/QuestionCard"; | |||
There was a problem hiding this comment.
The import path is incorrect. YesOrNo has been moved to the models directory and should be imported from "@site/src/models/yes-or-no" instead of from QuestionCard.
| import { YesOrNo } from "../components/navigator/QuestionCard"; | |
| import { YesOrNo } from "@site/src/models/yes-or-no"; |
| <AnsweredQuestion | ||
| key={index} | ||
| question={node.question || ""} | ||
| question={node.question} |
There was a problem hiding this comment.
The question property is optional in the DecisionNode interface (can be undefined), but here it's passed directly without a fallback. This creates an inconsistency with line 85 where the same property is safely handled with "|| ''". For type safety and consistency, this should also use a fallback.
| question={node.question} | |
| question={node.question || ""} |
| //Only call if the answer changed | ||
| if (answer !== "yes") { | ||
| onChangeAnswer("yes"); | ||
| } | ||
| }} | ||
| onAnswerNo={() => { | ||
| //Only call if the answer changed |
There was a problem hiding this comment.
Comments should have a space after the double slashes for better readability. Change "//Only" to "// Only".
| //Only call if the answer changed | |
| if (answer !== "yes") { | |
| onChangeAnswer("yes"); | |
| } | |
| }} | |
| onAnswerNo={() => { | |
| //Only call if the answer changed | |
| // Only call if the answer changed | |
| if (answer !== "yes") { | |
| onChangeAnswer("yes"); | |
| } | |
| }} | |
| onAnswerNo={() => { | |
| // Only call if the answer changed |
| //Only call if the answer changed | ||
| if (answer !== "yes") { | ||
| onChangeAnswer("yes"); | ||
| } | ||
| }} | ||
| onAnswerNo={() => { | ||
| //Only call if the answer changed |
There was a problem hiding this comment.
Comments should have a space after the double slashes for better readability. Change "//Only" to "// Only".
| //Only call if the answer changed | |
| if (answer !== "yes") { | |
| onChangeAnswer("yes"); | |
| } | |
| }} | |
| onAnswerNo={() => { | |
| //Only call if the answer changed | |
| // Only call if the answer changed | |
| if (answer !== "yes") { | |
| onChangeAnswer("yes"); | |
| } | |
| }} | |
| onAnswerNo={() => { | |
| // Only call if the answer changed |
| @@ -0,0 +1,36 @@ | |||
| import { YesOrNo } from "../components/navigator/QuestionCard"; | |||
| import { DecisionNode, decisionTree } from "../data/decision-tree"; | |||
There was a problem hiding this comment.
The import path is incorrect. DecisionNode has been moved to the models directory and should be imported from "@site/src/models/decision-node" instead of from the data/decision-tree file which no longer exports it.
| import { DecisionNode, decisionTree } from "../data/decision-tree"; | |
| import { DecisionNode } from "@site/src/models/decision-node"; | |
| import { decisionTree } from "../data/decision-tree"; |
This PR proposes to;