add new class option for cn, only show 1 button for cn class creation…#7915
add new class option for cn, only show 1 button for cn class creation…#7915
Conversation
… and open links in new tab
WalkthroughThis pull request introduces several enhancements to the CodeCombat platform, focusing on expanding classroom types, updating translations, and refining the teacher dashboard interface. The changes include adding new classroom configurations, introducing translations for game-based learning experiences like Ozaria and CodeCombat Junior, and modifying UI components to better support different user roles, particularly for CodeNinja users. Changes
Possibly related PRs
Poem
✨ Finishing Touches
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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
ozaria/site/components/teacher-dashboard/common/TitleBar.vue (2)
Line range hint
278-291: Consider differentiating tracking events for better analytics.Both buttons currently use the same tracking event name 'Add New Class Clicked'. Consider using distinct event names to better track CodeNinja vs regular user interactions.
- window.tracker?.trackEvent('Add New Class Clicked', { category: 'Teachers', label: this.$route.path }) + window.tracker?.trackEvent('Add New Club Clicked', { category: 'Teachers', label: this.$route.path })
Line range hint
278-291: Maintain consistent styling between buttons.The regular "Add Class" button includes the
dusk-btnclass while the CodeNinja "Add Club" button doesn't. Consider applying consistent styling to both buttons.<primary-button v-if="!showClassInfo && isCodeNinja" id="new-club-btn-shepherd" - class="btn-title-padding btn-margins-height" + class="btn-title-padding btn-margins-height dusk-btn" @click="clickNewClub" >ozaria/site/components/teacher-dashboard/BaseMyClasses/components/ClassLinksComponent.vue (1)
46-46: LGTM! Consider adding rel="noopener" for security.The addition of
target="_blank"to external links is a good practice. However, for enhanced security, consider addingrel="noopener"to prevent potential exploitation throughwindow.opener.Apply this pattern to all external links:
-target="_blank" +target="_blank" rel="noopener"Also applies to: 62-62, 78-78, 96-96, 111-111, 126-126, 141-141, 156-156, 176-176, 208-208, 226-226, 241-241, 256-256, 271-271, 286-286, 309-309, 339-339, 383-383, 443-443, 473-473, 517-517, 547-547, 561-561, 575-575, 589-589
ozaria/site/components/teacher-dashboard/modals/ModalEditClass.vue (1)
297-298: LGTM! Consider extracting magic numbers to constants.The validation for camp duration has been updated from 5 to 7 days. While the change is correct, consider extracting these magic numbers to named constants for better maintainability.
Consider defining constants at the top of the file:
+const CAMP_MAX_DAYS = 7 +const CLUB_MAX_WEEKS = 14 -} else if (this.newClubType.includes('camp') && moment(this.newClassDateEnd).diff(moment(this.newClassDateStart), 'days') > 7) { - errorMsg = 'Camp should be at most 7 days' +} else if (this.newClubType.includes('camp') && moment(this.newClassDateEnd).diff(moment(this.newClassDateStart), 'days') > CAMP_MAX_DAYS) { + errorMsg = `Camp should be at most ${CAMP_MAX_DAYS} days` -} else if (this.newClubType.includes('club') && moment(this.newClassDateEnd).diff(moment(this.newClassDateStart), 'weeks') > 14) { - errorMsg = 'Club should be at most 14 weeks' +} else if (this.newClubType.includes('club') && moment(this.newClassDateEnd).diff(moment(this.newClassDateStart), 'weeks') > CLUB_MAX_WEEKS) { + errorMsg = `Club should be at most ${CLUB_MAX_WEEKS} weeks`app/locale/en.js (1)
Line range hint
1000-1050: Consider refactoring long string concatenationsSeveral translations use string concatenation with + operators which can be hard to maintain. Consider using template literals or interpolation.
Example refactor:
-'faq_row3_p_part1': 'We customize solutions for schools and districts and work with you to understand your use case, context, and budget. ', -'faq_row3_p_part2': ' for further details! We also provide a ', +faq_row3_p: 'We customize solutions for schools and districts and work with you to understand your use case, context, and budget. For further details! We also provide a'
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
app/locale/en.js(1 hunks)app/models/Classroom.js(1 hunks)app/schemas/models/classroom.schema.js(1 hunks)ozaria/site/components/teacher-dashboard/BaseMyClasses/components/ClassLinksComponent.vue(25 hunks)ozaria/site/components/teacher-dashboard/common/TitleBar.vue(2 hunks)ozaria/site/components/teacher-dashboard/modals/ModalEditClass.vue(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: ESLint CI
- GitHub Check: Node.js CI (18.10.0)
🔇 Additional comments (7)
ozaria/site/components/teacher-dashboard/common/TitleBar.vue (1)
146-148: LGTM! Clean implementation of CodeNinja-specific logic.The addition of the CodeNinja check aligns well with the PR objectives to provide different UI experiences based on user roles.
app/schemas/models/classroom.schema.js (1)
123-123: LGTM! The new classroom type follows the schema's conventions.The addition of
annual-plan-cn-cocoto the classroom types is consistent with the existing enum values and aligns with the PR objectives.app/models/Classroom.js (1)
497-506: LGTM! Consider documenting the disabled types.The new classroom types are well-structured and follow the existing pattern. However, it would be helpful to understand the purpose of the disabled types (
club-3-month,camp-week, andannual-plan). Are these planned for future use?Let me check if these types are referenced elsewhere in the codebase:
app/locale/en.js (4)
Line range hint
1-10: LGTM - Module exports and nativeDescription setup looks goodThe basic structure with module.exports and initial translation setup follows best practices.
4713-4713:⚠️ Potential issueRemove stray line number
There appears to be a stray line number
4713~in the translations that should be removed.Likely invalid or redundant comment.
Line range hint
12-20: Verify HTML links in ozaria_home translationsThe links in the translations use HTML tags directly. Consider using a link helper/formatter for consistency and to avoid potential XSS issues.
Line range hint
2000-2050: Verify markdown link formattingSeveral translations use markdown-style links with ** for bold. Ensure these are being parsed correctly by the translation system.
1 participant
… and open links in new tab
Summary by CodeRabbit
Translations
New Features
User Interface
Validation