Catalyst Global Map Polishing #7954
Conversation
- Added a check to exclude Catalyst users from the anonymous classroom signup prompt, improving user experience and targeting.
- Added a condition to exclude Catalyst users from the anonymous classroom signup prompt, enhancing user targeting and experience.
…er experience - Increased font size and height for better visibility in the campaign view styles. - Adjusted margins and display properties for user status elements to improve alignment and responsiveness. - Simplified the Pug template by removing unnecessary wrapper elements, streamlining the structure.
WalkthroughThis pull request updates the visual styling and control flow within the campaign view. The SASS file now uses adjusted font sizes, heights, margins, and flex layouts for user status elements; the Pug template has been simplified by removing an extra wrapper for the player level; and the JavaScript view now incorporates additional conditions to bypass announcements and the anonymous classroom signup prompt when the Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant CV as CampaignView
U->>CV: Navigate to Campaign View
CV->>CV: Check isCatalyst in maybeShowPendingAnnouncement
alt isCatalyst true
CV-->>U: Do not display announcement
else
CV->>Announcement: Trigger pending announcement
Announcement-->>CV: Acknowledge display
CV-->>U: Show announcement
end
Assessment against linked issues
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms (2)
🪧 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 (
|
There was a problem hiding this comment.
Pull Request Overview
This PR refines the Catalyst campaign experience by removing certain popups and modals specific to Catalyst users and improving the top panel alignment.
- Removed the teacher popup and announcement modals for Catalyst.
- Adjusted logic to bypass anonymous classroom signup for Catalyst users.
Files not reviewed (2)
- app/styles/play/campaign-view.sass: Language not supported
- app/templates/play/campaign-view.pug: Language not supported
Comments suppressed due to low confidence (2)
app/views/play/CampaignView.js:2000
- Ensure that the property 'isCatalyst' is explicitly defined and documented within CampaignView to clarify its intended function and avoid any unintended behavior.
if (this.isCatalyst) { return false }
app/views/play/CampaignView.js:2178
- Confirm that including '!this.isCatalyst' in the anonymous classroom signup condition correctly prevents Catalyst users from triggering this functionality; adding a brief inline comment may improve clarity on its purpose.
return me.isAnonymous() && !this.isCatalyst &&
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
app/styles/play/campaign-view.sass (1)
1672-1681: New Keyframes "currentPracticeLevel" Introduced
This additional keyframes block introduces a pulse effect for practice level elements, transitioning the box-shadow and text color from white to a sky blue tone (#87CEFF). Verify that the effect complements the overall UI without becoming too distracting during extended usage.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
app/styles/play/campaign-view.sass(4 hunks)app/templates/play/campaign-view.pug(1 hunks)app/views/play/CampaignView.js(2 hunks)
🔇 Additional comments (11)
app/views/play/CampaignView.js (2)
1999-2000: Skip announcement modals for Catalyst usersThe condition added here prevents announcement modals from being shown to users in the Catalyst experiment, which aligns with the PR objective to remove announcement modals for the Catalyst feature.
2177-2180: Prevent classroom signup prompts for Catalyst usersThis change prevents anonymous users who are part of the Catalyst experiment from seeing the classroom signup dialog, which meets the PR objective to remove the teacher pop-up for the Catalyst feature.
app/templates/play/campaign-view.pug (1)
552-553: Improved top panel alignment by removing extra wrapperRemoving the
.rtl-alloweddiv wrapper around the player level elements simplifies the DOM structure and improves the alignment of elements in the top panel. This change directly addresses the PR objective to improve the alignment of the top panel.Note that this change still preserves the right-to-left text support as needed, since the individual spans still display the content correctly.
app/styles/play/campaign-view.sass (8)
22-31: New Keyframes "levelStartedPulse" Added
This new keyframes block defines a pulse effect using dynamic margin and box-shadow values. The use ofmath.divfor calculations is consistent with modern SASS practices. Please verify that these animations perform smoothly across target browsers.
928-933: Catalyst User-Status Adjustments
The.user-status-catalystblock now features an increased font size (26px) and updated dimensions (height and line-height raised to 40px) for a more spacious layout. Confirm that these new dimensions harmonize with other UI elements on various screen sizes.
940-943: Alignment Enhancement via Flexbox
Inside the catalyst’s user-status line, switching to a flex layout withdisplay: flex,align-items: center, andjustify-content: flex-endis a solid choice for improved alignment. This change should ensure that status elements line up neatly regardless of content changes.
958-960: Adjusted Positioning for Icons
Changing the positioning for.gem, .player-hero-iconfrom absolute to relative (withtop: auto) helps avoid unintended overlaps and misalignments. This is a thoughtful update to maintain consistency with the catalyst UI modifications.
963-965: Improved Alignment for Gems Count Display
The update to the#gems-countselector—reducingmargin-leftfrom 40px to 5px and setting it to usedisplay: inline-flexwith centered alignment—should integrate it more naturally with adjacent elements.
967-969: Enhanced Player Level Styling
Adjusting the.player-levelwith a reduced left margin and applyingdisplay: inline-flexwithalign-items: centerreinforces consistency with the overall text alignment in the catalyst status bar.
972-975: Refined Player Name Spacing and Alignment
Reducing the left margin to 5px and adding inline-flex styling with centered alignment for the.player-nameensures better visual coherence with the adjacent elements in the catalyst user-status.
992-995: Updated Hero Icon Scaling and Spacing
The changes to.player-hero-icon—increasing the side margins (to 0px 4px) and applying atransform: scale(1.33)withtransform-origin: center center—effectively scale the icon up from 30px to approximately 40px. This ensures the icon is more prominent and visually balanced in the catalyst header.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
app/styles/common/common.sass (1)
226-232: Descriptive Rename for Achievement Star Element
The updated class.achievements-staris clear and more descriptive than the previous generic.starclass. This improves readability and maintainability by clearly indicating the element's purpose.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
app/styles/common/common.sass(1 hunks)app/styles/play/campaign-view.sass(5 hunks)app/templates/play/campaign-view.pug(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- app/templates/play/campaign-view.pug
- app/styles/play/campaign-view.sass
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: ESLint CI
- GitHub Check: Node.js CI (18.10.0)
🔇 Additional comments (4)
app/styles/common/common.sass (4)
234-237: Consistent Modifier for 20px Achievement Star
The modifier&.achievements-star-20correctly sets the dimensions to 20px by 20px. This change is consistent with the new naming convention and ensures uniform styling for the smallest star variant.
238-241: Consistent Modifier for 30px Achievement Star
The modifier&.achievements-star-30appropriately defines a 30px by 30px size. It maintains consistency with other modifier classes and supports scalable UI elements for achievement stars.
242-245: Consistent Modifier for 40px Achievement Star
The modifier&.achievements-star-40specifies the element's dimensions at 40px by 40px. The uniform naming reinforces clarity across multiple size variants.
246-248: Consistent Modifier ACAD for 50px Achievement Star
The modifier&.achievements-star-50is correctly applied to render a 50px by 50px star element. The dimensional values are consistent with the intended design for larger achievement icons.
Closes GD-446, GD-567
Summary by CodeRabbit
Style
Refactor