Curriculum sidebar promotion #7593
Conversation
WalkthroughThe changes introduce a new 'curriculum guide' feature for teachers, enhancing navigation and accessibility within the application. Key updates include new routes, asynchronous data fetching for the curriculum guide, and the integration of a promotional modal. These enhancements aim to streamline the user experience for teachers accessing curriculum resources. Changes
Sequence Diagram(s)sequenceDiagram
participant Teacher as Teacher
participant UI as User Interface
participant Router as Router
participant Store as Vuex Store
participant Backend as Backend
Teacher->>UI: Navigate to Curriculum Guide
UI->>Router: Request /teachers/curriculum
Router->>UI: Load CurriculumGuide Component
UI->>Store: Fetch Curriculum Guide Data
Store->>Backend: Request Curriculum Data
Backend-->>Store: Respond with Data
Store-->>UI: Provide Curriculum Data
UI->>Teacher: Display Curriculum Guide
UI-->>Teacher: Show Promotion Modal (if not seen)
Assessment against linked issues
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (2)
app/assets/images/common/modal/curriculum-guide-screenshot.pngis excluded by!**/*.pngapp/assets/images/common/modal/right-arrow.pngis excluded by!**/*.png
Files selected for processing (11)
- app/core/Router.js (1 hunks)
- app/core/store/modules/teacherDashboard.js (2 hunks)
- app/core/vueRouter.js (1 hunks)
- app/locale/en.js (1 hunks)
- ozaria/site/components/teacher-dashboard/BaseCurriculumGuide/index.vue (7 hunks)
- ozaria/site/components/teacher-dashboard/BaseTeacherDashboard/index.vue (4 hunks)
- ozaria/site/components/teacher-dashboard/common/SecondaryTeacherNavigation.vue (10 hunks)
- ozaria/site/components/teacher-dashboard/common/TitleBar.vue (6 hunks)
- ozaria/site/components/teacher-dashboard/common/constants.js (3 hunks)
- ozaria/site/components/teacher-dashboard/modals/ModalCurriculumPromotion.vue (1 hunks)
- ozaria/site/components/teacher-dashboard/modals/ModalDynamicPromotion.vue (1 hunks)
Additional comments not posted (30)
ozaria/site/components/teacher-dashboard/common/constants.js (2)
12-12: LGTM! AddedCURRICULUM_GUIDEtoCOMPONENT_NAMES.The addition is consistent with the existing naming convention.
22-22: LGTM! Addedcurriculum_guidetoPAGE_TITLES.The addition is consistent with the existing naming convention.
ozaria/site/components/teacher-dashboard/modals/ModalDynamicPromotion.vue (1)
61-62: LGTM! Emitting 'show' event whenthis.showModalis true.The addition is consistent with the existing logic for handling modal visibility.
ozaria/site/components/teacher-dashboard/modals/ModalCurriculumPromotion.vue (1)
1-112: LGTM! NewModalCurriculumPromotioncomponent.The component is well-structured and consistent with the existing codebase.
ozaria/site/components/teacher-dashboard/BaseCurriculumGuide/index.vue (5)
4-4: LGTM! Added imports forCOMPONENT_NAMESandPAGE_TITLES.The imports are correct and necessary for the component.
14-14: LGTM! Set component name toCOMPONENT_NAMES.CURRICULUM_GUIDE.The addition is consistent with the existing naming convention.
59-62: LGTM! Addedmountedhook with necessary method calls.The additions are correct and necessary for the component's functionality.
67-73: LGTM! Updated actions and mutations.The additions are correct and necessary for the component's functionality.
Line range hint
99-173: LGTM! Updated template with new elements and styles.The additions are consistent with the existing codebase.
ozaria/site/components/teacher-dashboard/common/TitleBar.vue (3)
10-10: LGTM! Import changes are appropriate.The removal of
mapActionsand usage ofmapGettersis correct.
110-112: LGTM! Computed propertyinCurriculumis correctly implemented.The
inCurriculumproperty correctly checks if the current route path starts with/teachers/curriculum.
178-181: LGTM! Template changes are correctly implemented.The class
teacher-title-baris conditionally applied based oninCurriculum.app/core/store/modules/teacherDashboard.js (2)
216-218: LGTM! Changes in thefetchDataaction are correctly implemented.The new conditional block for
COMPONENT_NAMES.CURRICULUM_GUIDEcorrectly dispatchesfetchDataCurriculumGuideAsync.
367-372: LGTM! The newfetchDataCurriculumGuideAsyncaction is correctly implemented.The action correctly dispatches actions to fetch prepaids for the teacher and curriculum guide data asynchronously.
ozaria/site/components/teacher-dashboard/common/SecondaryTeacherNavigation.vue (8)
7-7: LGTM! Import ofModalCurriculumPromotionis correctly added.The import is correctly added and used in the component.
27-32: LGTM! Data propertiesshowCurriculumPromotionandcurriculumPromoClickedare correctly added.The data properties are correctly added and used in the component.
109-115: LGTM! MethodonCurriculumClickedis correctly implemented.The method correctly handles curriculum promotion interactions.
133-137: LGTM! MethodshighlightCurriculumandunhighlightCurriculumPromotionare correctly implemented.The methods correctly handle curriculum promotion interactions.
132-132: LGTM! Modifications in theonHackStackClickedmethod are correctly implemented.The modifications correctly handle curriculum promotion interactions when the HackStack is clicked.
252-263: LGTM! Template changes forCurriculumAnchorare correctly implemented.The changes correctly integrate the curriculum promotion feature into the template.
434-438: LGTM! Template changes forModalCurriculumPromotionare correctly implemented.The changes correctly integrate the curriculum promotion feature into the template.
504-515: LGTM! CSS styles forCurriculumAnchorandIconCurriculumare correctly added.The styles are correctly added to support the new curriculum promotion feature.
ozaria/site/components/teacher-dashboard/BaseTeacherDashboard/index.vue (3)
Line range hint
1-1: LGTM! Import changes are correctly implemented.The import and usage of
BaseCurriculumGuidecomponent have been correctly removed.
Line range hint
1-1: LGTM! Method changes are correctly implemented.The
closeCurriculumGuidemethod and its corresponding call in thebeforeRouteUpdatehook have been correctly removed.
Line range hint
1-1: LGTM! Template changes are correctly implemented.The
<base-curriculum-guide>component has been correctly removed from the template.app/core/vueRouter.js (1)
274-274: LGTM!The new route for 'curriculum' is correctly configured and follows best practices.
app/core/Router.js (1)
487-487: LGTM!The new route for 'teachers/curriculum' is correctly configured and follows best practices.
app/locale/en.js (3)
1776-1776: LGTM!The added string clearly conveys the curriculum's capabilities and its beta status.
1777-1777: LGTM!The added string serves as a clear header for the dashboard update message.
1778-1778: LGTM!The added string provides a clear and informative update about the dashboard optimization and the new location of the Curriculum Guide.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- app/schemas/models/user.js (1 hunks)
- ozaria/site/components/teacher-dashboard/common/SecondaryTeacherNavigation.vue (10 hunks)
- ozaria/site/components/teacher-dashboard/modals/ModalCurriculumPromotion.vue (1 hunks)
- ozaria/site/components/teacher-dashboard/modals/ModalHackStackBeta.vue (2 hunks)
Files skipped from review as they are similar to previous changes (2)
- ozaria/site/components/teacher-dashboard/common/SecondaryTeacherNavigation.vue
- ozaria/site/components/teacher-dashboard/modals/ModalCurriculumPromotion.vue
Additional comments not posted (5)
ozaria/site/components/teacher-dashboard/modals/ModalHackStackBeta.vue (4)
27-27: LGTM!The
onTryItNowmethod correctly emits an event, closes the modal, and tracks the event.
28-34: LGTM!The
showSignalmethod correctly emitsshoworcloseevents based on theshowModalstate.
35-36: LGTM!The
onClosemethod correctly emits thecloseevent.
48-49: LGTM!The template correctly includes the
ModalDynamicPromotioncomponent with conditional rendering and event handling.app/schemas/models/user.js (1)
410-410: LGTM!The addition of the
curriculum-sidebar-promotion-modalproperty to theseenPromotionsobject in theUserSchemais correct.
|
Look great, thanks! |
| this.hackstackModalVisibility = true | ||
| }, | ||
| hackstackModalClose () { | ||
| console.log('hackstack close') |
| watch: { | ||
| curriculumClicked (newVal, oldVal) { | ||
| if (newVal) { | ||
| this.showModal = false |
There was a problem hiding this comment.
If we hide the modal this way, the seen flag won't be set, and modal will appear again.
Let's call the modal's close method instead:
| this.showModal = false | |
| this.$refs.modal.close() |
| @@ -226,6 +257,18 @@ export default { | |||
| </li> | |||
| </ul> | |||
| </li> | |||
| <li :class="{ 'modal-highlight': showCurriculumPromotion && !hackstackModalVisibility }"> | |||
There was a problem hiding this comment.
If other modals are added, they'll also should be checked. The next is coming soon.
I'm not sure, but maybe there's a way to check if the curriculum modal is shown instead of checking if all the others are not visible?
There was a problem hiding this comment.
our logic of showing is controlled by modal, if parent had a control over it. That would have made it easier to handle this one.
We should at least group the condition to show modals in a mixin like we collect redis keys in backend so that we know what modals are there and what will show up to control other modals.
There was a problem hiding this comment.
should I wait for the test as student promotion modal to get merged so it would be easier to change the logic?
There was a problem hiding this comment.
can you tell me where the files are for collecting redis keys in the backend? I'm still not sure how to implment this.
| computed: { | ||
| isOld () { | ||
| const twoDaysAgo = new Date(new Date() - 2 * 24 * 60 * 60 * 1000) | ||
| return new Date(me.get('dateCreated')) < twoDaysAgo |
There was a problem hiding this comment.
let's also not show it dateCreated is after august 7. We don't need it for new teachers anyway.
There was a problem hiding this comment.
@gabriellui1 can you just implement this change for the PR? We will worry about other changes in a different feature.
| @@ -226,6 +257,18 @@ export default { | |||
| </li> | |||
| </ul> | |||
| </li> | |||
| <li :class="{ 'modal-highlight': showCurriculumPromotion && !hackstackModalVisibility }"> | |||
There was a problem hiding this comment.
our logic of showing is controlled by modal, if parent had a control over it. That would have made it easier to handle this one.
We should at least group the condition to show modals in a mixin like we collect redis keys in backend so that we know what modals are there and what will show up to control other modals.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- ozaria/site/components/teacher-dashboard/common/SecondaryTeacherNavigation.vue (10 hunks)
- ozaria/site/components/teacher-dashboard/modals/ModalCurriculumPromotion.vue (1 hunks)
Additional comments not posted (16)
ozaria/site/components/teacher-dashboard/modals/ModalCurriculumPromotion.vue (5)
37-41: Verify the logic for determining old users.Ensure that the logic for determining old users is accurate and aligns with the requirements.
45-47: Verify the close event handling.Ensure that the close event is handled properly and the modal does not reappear unexpectedly.
29-33: Verify theseenflag setting.Ensure that the
seenflag is set properly when the modal is closed.
22-25: Verify the date comparison logic.Ensure that the date comparison logic accurately determines if the user is old.
53-79: Verify the modal display and hide logic.Ensure that the modal is displayed and hidden properly based on the
showModalproperty and events.ozaria/site/components/teacher-dashboard/common/SecondaryTeacherNavigation.vue (11)
110-114: Verify the unhighlighting logic.Ensure that the curriculum promotion is unhighlighted properly when the curriculum is clicked.
134-136: Verify the highlighting logic.Ensure that the curriculum promotion is highlighted properly when the function is called.
137-139: Verify the unhighlighting logic.Ensure that the curriculum promotion is unhighlighted properly when the function is called.
140-141: Verify the hackstack modal visibility logic.Ensure that the hackstack modal visibility is set properly when the function is called.
143-144: Verify the hackstack modal visibility logic.Ensure that the hackstack modal visibility is set properly when the function is called.
Line range hint
117-125:
Verify the event tracking logic.Ensure that the event tracking logic is accurate and events are tracked properly.
29-29: Verify the usage ofshowCurriculumPromotion.Ensure that the
showCurriculumPromotiondata property is used properly to control the visibility of the curriculum promotion.
30-30: Verify the usage ofcurriculumPromoClicked.Ensure that the
curriculumPromoClickeddata property is used properly to track if the curriculum promotion was clicked.
31-31: Verify the usage ofhackstackModalVisibility.Ensure that the
hackstackModalVisibilitydata property is used properly to control the visibility of the hackstack modal.
441-444: Verify the modal display and hide logic.Ensure that the curriculum promotion modal is displayed and hidden properly based on the
curriculumPromoClickedproperty and events.
259-269: Verify the link highlighting logic.Ensure that the curriculum anchor link is highlighted properly based on the
showCurriculumPromotionandhackstackModalVisibilityproperties.
b8ffa3c to
7df3f97
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (13)
- app/core/Router.js (1 hunks)
- app/core/store/modules/teacherDashboard.js (2 hunks)
- app/core/vueRouter.js (1 hunks)
- app/locale/en.js (1 hunks)
- app/schemas/models/user.js (1 hunks)
- ozaria/site/components/teacher-dashboard/BaseCurriculumGuide/index.vue (7 hunks)
- ozaria/site/components/teacher-dashboard/BaseTeacherDashboard/index.vue (4 hunks)
- ozaria/site/components/teacher-dashboard/common/SecondaryTeacherNavigation.vue (10 hunks)
- ozaria/site/components/teacher-dashboard/common/TitleBar.vue (6 hunks)
- ozaria/site/components/teacher-dashboard/common/constants.js (3 hunks)
- ozaria/site/components/teacher-dashboard/modals/ModalCurriculumPromotion.vue (1 hunks)
- ozaria/site/components/teacher-dashboard/modals/ModalDynamicPromotion.vue (1 hunks)
- ozaria/site/components/teacher-dashboard/modals/ModalHackStackBeta.vue (2 hunks)
Files skipped from review due to trivial changes (2)
- app/locale/en.js
- ozaria/site/components/teacher-dashboard/common/constants.js
Files skipped from review as they are similar to previous changes (1)
- ozaria/site/components/teacher-dashboard/modals/ModalCurriculumPromotion.vue
Additional comments not posted (28)
ozaria/site/components/teacher-dashboard/modals/ModalDynamicPromotion.vue (1)
61-63: LGTM! Event emission enhances interactivity.The addition of the
$emit('show')event in thecreatedlifecycle method improves the component's interactivity by allowing parent components to respond to the modal's visibility state.ozaria/site/components/teacher-dashboard/modals/ModalHackStackBeta.vue (2)
28-33: LGTM! New method improves responsiveness.The addition of the
showSignalmethod enhances the component's responsiveness to modal visibility changes by emitting 'show' or 'close' events based on the state ofshowModal.
48-49: LGTM! Event listeners improve communication.The addition of
@showand@closeevent listeners in the template enhances the communication between the modal and its parent component.ozaria/site/components/te 9E81 acher-dashboard/BaseCurriculumGuide/index.vue (4)
4-4: LGTM! New imports improve maintainability.The addition of
COMPONENT_NAMESandPAGE_TITLESimports enhances the maintainability and clarity of the code by centralizing these values.
59-63: LGTM! New lifecycle method improves initialization.The addition of the
mountedlifecycle method enhances the component's initialization process by setting the teacher ID, assigning the page title, and fetching essential data immediately upon rendering.
67-73: LGTM! New methods improve state management.The addition of new actions and mutations related to the teacher dashboard reflects a shift towards a more centralized state management approach via Vuex, enhancing the maintainability and scalability of the code.
99-147: LGTM! Template changes enhance UI/UX.The structural changes in the template section, including the removal of the transition wrapper and updates to the CSS, simplify the DOM structure and enhance the visual appeal and user experience.
ozaria/site/components/teacher-dashboard/common/TitleBar.vue (2)
110-112: LGTM!The
inCurriculummethod is correctly implemented to check if the current route path starts with/teachers/curriculum.
178-181: LGTM!The template changes correctly utilize the
inCurriculummethod to conditionally render the title bar.app/core/store/modules/teacherDashboard.js (2)
216-218: LGTM!The
fetchDatamethod correctly integrates the newfetchDataCurriculumGuideAsyncmethod for theCOMPONENT_NAMES.CURRICULUM_GUIDEcase.
367-372: LGTM!The
fetchDataCurriculumGuideAsyncmethod is correctly implemented to fetch prepaids for a teacher and curriculum guide data concurrently usingPromise.all.ozaria/site/components/teacher-dashboard/BaseTeacherDashboard/index.vue (3)
Line range hint
1-23: LGTM!The removal of the
BaseCurriculumGuidecomponent from the imports is straightforward and aligns with the objective of the PR.
Line range hint
62-66: LGTM!The removal of the
closeCurriculumGuidemethod from the methods is straightforward and aligns with the objective of the PR.
Line range hint
178-181: LGTM!The removal of the
<base-curriculum-guide>element from the template is straightforward and aligns with the objective of the PR.ozaria/site/components/teacher-dashboard/common/SecondaryTeacherNavigation.vue (11)
7-7: LGTM!The import statement for
ModalCurriculumPromotionlooks correct.
27-32: LGTM!The new reactive properties
showCurriculumPromotion,curriculumPromoClicked, andhackstackModalVisibilityare correctly defined and initialized.
110-116: LGTM!The
onCurriculumClickedmethod correctly handles the curriculum promotion state and tracks events.
134-136: LGTM!The
highlightCurriculummethod correctly setsshowCurriculumPromotionto true.
137-139: LGTM!The
unhighlightCurriculumPromotionmethod correctly setsshowCurriculumPromotionto false.
140-142: LGTM!The
hackstackModalShowingmethod correctly setshackstackModalVisibilityto true.
143-145: LGTM!The
hackstackModalClosemethod correctly setshackstackModalVisibilityto false.
259-270: LGTM!The template section for the curriculum link correctly handles user interactions and state.
441-451: LGTM!The modal components
ModalCurriculumPromotionandModalHackStackBetaare correctly added to the template and follow best practices for managing props and events.
513-524: LGTM!The new CSS rules for hover effects and background images for the curriculum icon are correctly added and follow best practices for managing visual indicators.
770-772: LGTM!The new CSS rule for
.modal-highlightcorrectly manages the z-index.app/core/vueRouter.js (1)
274-274: LGTM!The new route for the curriculum guide is correctly added and follows best practices for adding routes in Vue Router.
app/schemas/models/user.js (1)
410-410: LGTM!The new property
curriculum-sidebar-promotion-modalis correctly added and follows best practices for extending the schema.app/core/Router.js (1)
487-487: LGTM! New route added forteachers/curriculum.The new route is correctly added and uses the
teacherProxyRouteandgofunctions to handle requests.
fixes ENG-943
Same logic for whether users can see or not see as this: #7562
Summary by CodeRabbit
New Features
Improvements
UI/UX Enhancements