-
Notifications
You must be signed in to change notification settings - Fork 13
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
Use CodeMirror
in FileContentsCard
instead of SyntaxHighlightedCode
#1907
Use CodeMirror
in FileContentsCard
instead of SyntaxHighlightedCode
#1907
Conversation
WalkthroughThe recent changes introduce enhancements to the 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 (
|
CodeMirror
in FileContentsCard
instead of SyntaxHighlightedCode
componentCodeMirror
in FileContentsCard
instead of SyntaxHighlightedCode
Codecov ReportAttention: Patch coverage is ✅ All tests successful. No failed tests found. Additional details and impacted files📢 Thoughts on this report? Let us know! |
ec7486f
to
82c3f7e
Compare
Bundle ReportChanges will decrease total bundle size by 168.49kB ⬇️
|
903090b
to
fbdf036
Compare
@VasylMarchuk can we try to change the styling so that this matches the current version more closely? Let's remove the vertical border, adjust spacing + also remove the blue highlight on the selected line (this I'm assuming would just be a codemirror option to disable?) |
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.
Added note re: styling + handling unchanged files in community solutions
app/components/course-page/course-stage-step/community-solution-card/content.hbs
Outdated
Show resolved
Hide resolved
cfab745
to
a289eb2
Compare
29d1693
to
04b0704
Compare
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
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
package-lock.json
is excluded by!**/package-lock.json
Files selected for processing (18)
- app/components/code-mirror.ts (9 hunks)
- app/components/course-page/course-stage-step/community-solution-card/content.hbs (2 hunks)
- app/components/file-contents-card.hbs (2 hunks)
- app/components/file-contents-card.ts (3 hunks)
- app/controllers/demo/code-mirror.ts (3 hunks)
- app/controllers/demo/file-contents-card.ts (1 hunks)
- app/router.ts (1 hunks)
- app/routes/demo/file-contents-card.ts (1 hunks)
- app/templates/course-admin/buildpack.hbs (1 hunks)
- app/templates/demo.hbs (1 hunks)
- app/templates/demo/code-mirror.hbs (6 hunks)
- app/templates/demo/file-contents-card.hbs (1 hunks)
- app/utils/code-mirror-themes.ts (1 hunks)
- package.json (1 hunks)
- tests/acceptance/course-admin/view-code-example-test.js (1 hunks)
- tests/integration/components/code-mirror-test.js (1 hunks)
- tests/pages/components/code-mirror.ts (1 hunks)
- tests/pages/course-page.js (2 hunks)
Files skipped from review as they are similar to previous changes (15)
- app/components/code-mirror.ts
- app/components/course-page/course-stage-step/community-solution-card/content.hbs
- app/components/file-contents-card.hbs
- app/components/file-contents-card.ts
- app/controllers/demo/code-mirror.ts
- app/router.ts
- app/templates/course-admin/buildpack.hbs
- app/templates/demo.hbs
- app/templates/demo/code-mirror.hbs
- app/templates/demo/file-contents-card.hbs
- app/utils/code-mirror-themes.ts
- package.json
- tests/acceptance/course-admin/view-code-example-test.js
- tests/integration/components/code-mirror-test.js
- tests/pages/course-page.js
Additional comments not posted (2)
app/routes/demo/file-contents-card.ts (1)
1-3
: Basic Route Setup Looks GoodThe class
DemoFileContentsCardRoute
is defined correctly for a basic Ember.js route. It appears to be a placeholder for future expansions or specific route handling.tests/pages/components/code-mirror.ts (1)
1-29
: Comprehensive Test Setup for CodeMirror ComponentThe page object setup for the
CodeMirror
component is well-structured and covers a wide range of interactions which are essential for thorough testing. The use of aliases enhances the readability and reusability of the code.
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
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- app/components/file-contents-card.hbs (2 hunks)
- app/components/file-contents-card.ts (1 hunks)
- app/templates/demo/file-contents-card.hbs (1 hunks)
Files skipped from review as they are similar to previous changes (3)
- app/components/file-contents-card.hbs
- app/components/file-contents-card.ts
- app/templates/demo/file-contents-card.hbs
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: 2
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- app/components/code-mirror.ts (9 hunks)
- app/controllers/demo/code-mirror.ts (3 hunks)
- tests/integration/components/code-mirror-test.js (2 hunks)
- tests/pages/course-page.js (2 hunks)
Files skipped from review as they are similar to previous changes (3)
- app/components/code-mirror.ts
- tests/integration/components/code-mirror-test.js
- tests/pages/course-page.js
Additional context used
Biome
app/controllers/demo/code-mirror.ts
[error] 152-152: This return should return a value because it is located in a getter.
(lint/suspicious/useGetterReturn)
Additional comments not posted (3)
app/controllers/demo/code-mirror.ts (3)
14-15
: Ensure the new themes are properly added.The addition of
codeCraftersLight
andcodeCraftersDark
toTHEME_EXTENSIONS
is noted. Ensure that these themes are properly defined and imported as they are crucial for the functionality of theCodeMirror
component.
113-113
: Review the initialization of theme-related properties.The initialization of
themes
usingSUPPORTED_THEMES
andselectedThemeIndex
using the default theme index are crucial for ensuring that the theme functionality works as expected. Confirm that these properties are correctly utilized throughout the component.Also applies to: 119-119
18-19
: Verify the new theme support.The addition of
'codeCraftersAuto'
toSUPPORTED_THEMES
and its setting as theDEFAULT_THEME
are significant changes. Verify that this theme dynamically adjusts according to the system's theme settings.Verification successful
Verified: The
codeCraftersAuto
theme dynamically adjusts according to the system's theme settings.The logic in the
app/controllers/demo/code-mirror.ts
file confirms that thecodeCraftersAuto
theme switches betweencodeCraftersDark
andcodeCraftersLight
based on the system's dark mode setting.
app/controllers/demo/code-mirror.ts
: Lines showing the dynamic adjustment logic.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that 'codeCraftersAuto' dynamically adjusts according to the system's theme settings. # Test: Search for the theme adjustment logic. rg --type python -A 5 $'codeCraftersAuto'Length of output: 71
Script:
#!/bin/bash # Description: Verify that 'codeCraftersAuto' dynamically adjusts according to the system's theme settings. # Test: Search for the theme adjustment logic in TypeScript files. rg --type typescript -A 10 'codeCraftersAuto'Length of output: 80
Script:
#!/bin/bash # Description: Verify that 'codeCraftersAuto' dynamically adjusts according to the system's theme settings. # Test: Search for the theme adjustment logic. rg -A 10 'codeCraftersAuto'Length of output: 1484
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
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- app/controllers/demo/code-mirror.ts (3 hunks)
Files skipped from review as they are similar to previous changes (1)
- app/controllers/demo/code-mirror.ts
Related to #1231 #1011
Brief
This updates
FileContentsCard
to useCodeMirror
instead ofSyntaxHighlightedCode
component.Details
FileContentsCard
now usesCodeMirror
instead ofSyntaxHighlightedCode
FileContentsCard
componentFileContentsCard
componentFileContentsCard
componentFileContentsCard
currently appears in the following places:courses/redis/admin/buildpacks/buildpack
routeapp/components/course-page/course-stage-step/community-solution-card/content.hbs
template/demo/file-contents-card
for showcasingFileContentsCard
component@theme
toCodeMirror
componentcodeCraftersLight
andcodeCraftersDark
course-admin | view-code-example
@ember/test-waiters
package to the projectScreenshots
Checklist
[percy]
in the message to trigger)Summary by CodeRabbit
New Features
codeCraftersLight
andcodeCraftersDark
for enhanced theme customization.codeCraftersAuto
.Enhancements
Bug Fixes
Tests