-
Notifications
You must be signed in to change notification settings - Fork 4.1k
CodeCombat Junior Art Updates #7519
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Important Review skippedReview was skipped due to path filters Files ignored due to path filters (1)
You can disable this status message by setting the WalkthroughThe updates enhance the visual presentation and functionality of the Blockly interface and various elements within the app. Notable changes include conditional adjustments in Blockly toolbox and block setup, styling improvements in several SASS files, a template image update, removal of import confirmations, and refined goal rendering logic in Vue components. These modifications collectively aim to enrich user interaction and visual clarity across the application. Changes
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 Configration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Outside diff range and nitpick comments (4)
app/views/editor/thang/ThangTypeEditView.js (4)
Line range hint
160-165
: Usingthis
in a static context can be confusing and potentially error-prone.Consider using the class name instead of
this
to access static properties or methods.
Line range hint
255-303
: Assignments within expressions can lead to confusion and are generally considered bad practice.Consider separating assignments from expressions for clarity and maintainability.
Line range hint
460-463
: The else clause here is unnecessary as previous branches break early.Consider removing the else clause to simplify the flow of control.
Line range hint
703-703
: Using the delete operator can impact performance and lead to unpredictable behaviors in some cases.Consider setting the property to
undefined
or restructuring the code to avoid the need for deletion.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (11)
app/assets/images/level/goal-icons/checkmark.png
is excluded by!**/*.png
app/assets/images/level/goal-icons/clean-code.png
is excluded by!**/*.png
app/assets/images/level/goal-icons/collect-thangs.png
is excluded by!**/*.png
app/assets/images/level/goal-icons/get-to-locations-raft.png
is excluded by!**/*.png
app/assets/images/level/goal-icons/heart-empty.png
is excluded by!**/*.png
app/assets/images/level/goal-icons/heart.png
is excluded by!**/*.png
app/assets/images/level/goal-icons/kill-thangs.png
is excluded by!**/*.png
app/assets/images/level/goal-icons/lines-of-code.png
is excluded by!**/*.png
app/assets/images/level/goal-icons/red-x.png
is excluded by!**/*.png
app/assets/images/level/goal-icons/save-thangs-empty.png
is excluded by!**/*.png
app/assets/images/level/goal-icons/save-thangs.png
is excluded by!**/*.png
Files selected for processing (3)
- app/views/editor/thang/ThangTypeEditView.js (1 hunks)
- app/views/play/level/LevelGoal.vue (7 hunks)
- app/views/play/level/LevelGoals.vue (2 hunks)
Files skipped from review due to trivial changes (1)
- app/views/play/level/LevelGoals.vue
Additional context used
Biome
app/views/editor/thang/ThangTypeEditView.js
[error] 160-160: Using this in a static context can be confusing. (lint/complexity/noThisInStatic)
this refers to the class.
Unsafe fix: Use the class name instead.
[error] 161-161: Using this in a static context can be confusing. (lint/complexity/noThisInStatic)
this refers to the class.
Unsafe fix: Use the class name instead.
[error] 162-162: Using this in a static context can be confusing. (lint/complexity/noThisInStatic)
this refers to the class.
Unsafe fix: Use the class name instead.
[error] 163-163: Using this in a static context can be confusing. (lint/complexity/noThisInStatic)
this refers to the class.
Unsafe fix: Use the class name instead.
[error] 164-164: Using this in a static context can be confusing. (lint/complexity/noThisInStatic)
this refers to the class.
Unsafe fix: Use the class name instead.
[error] 165-165: Using this in a static context can be confusing. (lint/complexity/noThisInStatic)
this refers to the class.
Unsafe fix: Use the class name instead.
[error] 172-172: Using this in a static context can be confusing. (lint/complexity/noThisInStatic)
this refers to the class.
Unsafe fix: Use the class name instead.
[error] 200-200: Using this in a static context can be confusing. (lint/complexity/noThisInStatic)
this refers to the class.
Unsafe fix: Use the class name instead.
[error] 255-255: The assignment should not be in an expression. (lint/suspicious/noAssignInExpressions)
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.
[error] 287-287: The assignment should not be in an expression. (lint/suspicious/noAssignInExpressions)
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.
[error] 303-303: The assignment should not be in an expression. (lint/suspicious/noAssignInExpressions)
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.
[error] 406-406: The assignment should not be in an expression. (lint/suspicious/noAssignInExpressions)
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.
[error] 460-463: This else clause can be omitted because previous branches break early. (lint/style/noUselessElse)
Unsafe fix: Omit the else clause.
[error] 592-592: The assignment should not be in an expression. (lint/suspicious/noAssignInExpressions)
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.
[error] 618-618: The assignment should not be in an expression. (lint/suspicious/noAssignInExpressions)
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.
[error] 681-681: The assignment should not be in an expression. (lint/suspicious/noAssignInExpressions)
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.
[error] 696-696: The assignment should not be in an expression. (lint/suspicious/noAssignInExpressions)
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.
[error] 703-703: Avoid the delete operator which can impact performance. (lint/performance/noDelete)
Unsafe fix: Use an undefined assignment instead.
[error] 719-719: The assignment should not be in an expression. (lint/suspicious/noAssignInExpressions)
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.
[error] 742-742: The assignment should not be in an expression. (lint/suspicious/noAssignInExpressions)
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.
Additional comments not posted (6)
app/views/play/level/LevelGoal.vue (6)
22-35
: Updated icon mappings are well-reflected in the code.
65-66
: This loop for adding heart icons is efficient and clear.
106-106
: MethodiconImageSrc
is a clean implementation for handling dynamic icon sources based on state.
117-124
: Styling changes support the new UI elements well, ensuring a consistent look and feel.
139-143
: Positioning and sizing of the goal icon status are well-defined, enhancing UI clarity.
4-4
: EnsureiconImageSrc
is always defined when used here to avoid runtime errors.
for key, value of goalIconMap when @goal[key] and not text | ||
text = value | ||
else | ||
if @product isnt 'codecombat-junior' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Complex conditional logic detected. Consider simplifying or adding comments for clarity.
- if @product isnt 'codecombat-junior'
+ # Check if product is not 'codecombat-junior' and adjust the text accordingly
+ if @product !== 'codecombat-junior'
Committable suggestion was skipped due to low confidence.
Also remove extraneous "Hero" category name, update dropdown argument colors, make block icons a little bigger, and use directional arrows instead of text in non-icon block dropdowns.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Outside diff range and nitpick comments (4)
app/core/blocklyUtils.js (4)
Line range hint
62-64
: Convert function expressions to arrow functions.- function (block) { + (block) => {Convert all function expressions in this file to arrow functions where
this
is not used, to improve readability and align with modern JavaScript practices.Also applies to: 77-82, 93-97, 109-114, 149-167, 265-274, 292-297, 313-318, 334-339, 366-374, 15-553, 563-627, 553-813, 813-826, 827-833, 834-860, 861-884
Line range hint
235-235
: Remove comma operator usage.- const OPERATORS = { + const OPERATORS = [The comma operator is often confusing and should be replaced with semicolons or other appropriate syntax to clearly separate statements.
Line range hint
269-272
: Remove unnecessary 'else' clause.- } else { - return false; - }Since the previous branches of the conditional already terminate the function with a return statement, the 'else' clause is unnecessary and can be omitted to simplify the control flow.
Line range hint
865-883
: Address unreachable code.Remove or refactor the unreachable code to ensure that all parts of the function can be executed as intended. This may involve rethinking the logic of the function to make it more straightforward or removing redundant conditions.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (14)
app/assets/images/level/blocks/block-down.png
is excluded by!**/*.png
app/assets/images/level/blocks/block-false.png
is excluded by!**/*.png
app/assets/images/level/blocks/block-go.png
is excluded by!**/*.png
app/assets/images/level/blocks/block-hit.png
is excluded by!**/*.png
app/assets/images/level/blocks/block-if.png
is excluded by!**/*.png
app/assets/images/level/blocks/block-left.png
is excluded by!**/*.png
app/assets/images/level/blocks/block-look.png
is excluded by!**/*.png
app/assets/images/level/blocks/block-loop.png
is excluded by!**/*.png
app/assets/images/level/blocks/block-right.png
is excluded by!**/*.png
app/assets/images/level/blocks/block-spin.png
is excluded by!**/*.png
app/assets/images/level/blocks/block-true.png
is excluded by!**/*.png
app/assets/images/level/blocks/block-up.png
is excluded by!**/*.png
app/assets/images/level/blocks/block-zap.png
is excluded by!**/*.png
app/assets/images/level/goal-icons/next-level.png
is excluded by!**/*.png
Files selected for processing (6)
- app/core/blocklyUtils.js (5 hunks)
- app/styles/play/common/blockly.sass (1 hunks)
- app/styles/play/level/tome/cast_button.sass (1 hunks)
- app/templates/play/level/modal/hero-victory-modal.pug (1 hunks)
- app/views/play/level/LevelGoal.vue (7 hunks)
- app/views/play/level/LevelGoals.vue (2 hunks)
Files skipped from review due to trivial changes (2)
- app/styles/play/common/blockly.sass
- app/templates/play/level/modal/hero-victory-modal.pug
Files skipped from review as they are similar to previous changes (2)
- app/views/play/level/LevelGoal.vue
- app/views/play/level/LevelGoals.vue
Additional context used
Biome
app/core/blocklyUtils.js
[error] 62-64: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)
Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.
[error] 77-82: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)
Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.
[error] 93-97: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)
Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.
[error] 109-114: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)
Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.
[error] 149-167: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)
Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.
[error] 235-235: The comma operator is disallowed. (lint/style/noCommaOperator)
Its use is often confusing and obscures side effects.
[error] 269-272: This else clause can be omitted because previous branches break early. (lint/style/noUselessElse)
Unsafe fix: Omit the else clause.
[error] 265-274: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)
Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.
[error] 292-297: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)
Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.
[error] 313-318: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)
Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.
[error] 334-339: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)
Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.
[error] 366-374: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)
Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.
[error] 15-553: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)
Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.
[error] 563-627: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)
Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.
[error] 553-813: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)
Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.
[error] 813-826: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)
Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.
[error] 827-833: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)
Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.
[error] 834-860: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)
Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.
[error] 861-884: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)
Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.
[error] 865-883: This code is unreachable (lint/correctness/noUnreachable)
... because either this statement ...
... or this statement will return from the function beforehand
Additional comments not posted (4)
app/styles/p 8000 lay/level/tome/cast_button.sass (4)
Line range hint
3-13
: IntroducecastablePulse
keyframe animation to enhance UI feedback.Nice addition of the
castablePulse
animation for visual feedback. This should help make the button interactions more engaging for the user.
Line range hint
15-25
: IntroducewinnablePulse
keyframe animation to enhance UI feedback.The
winnablePulse
animation is a good visual indicator for winnable states, improving user experience by providing dynamic feedback.
126-130
: Enhance button visibility and interaction.Increasing the opacity and adjusting the brightness on hover/castable states for
.cast-button
enhances user interaction by making the button more noticeable and responsive.
135-135
: Increase icon visibility.The increase in font size for
.glyphicon
makes the icons more visible, which is beneficial for accessibility and usability.
We replaced the goal ThangType with raft art.
Update Junior goals icons and display
Update Junior block icons
Summary by CodeRabbit
New Features
Improvements
User Experience
.js
files, streamlining the import process.