Skip to content

feat(header): enhance track page header layout and content #2597

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

rohitpaulk
Copy link
Member

@rohitpaulk rohitpaulk commented Jan 29, 2025

Refactor the track page header to improve layout and visual
consistency. Adjust the structure to better align elements and
add additional statistics for learners, exercises, concepts,
and questions. Update test scenarios to reflect changes in
track identifiers and remove unnecessary assertions.

Summary by CodeRabbit

  • UI/Layout Changes

    • Restructured track page header layout.
    • Added new statistics display with icons.
    • Removed previous top participants section.
    • Updated Rust logo display.
    • Enhanced track page with new "Rust Primer" section and concept items.
    • Simplified page divider styling.
  • Test Updates

    • Commented out multiple track page test cases.
    • Modified test configuration for Rust track.

Refactor the track page header to improve layout and visual 
consistency. Adjust the structure to better align elements and 
add additional statistics for learners, exercises, concepts, 
and questions. Update test scenarios to reflect changes in 
track identifiers and remove unnecessary assertions.
Copy link
Contributor

coderabbitai bot commented Jan 29, 2025

Walkthrough

This pull request introduces substantial changes to the track page header in the Exercism web application. The modifications focus on restructuring the header layout, replacing user interaction buttons with a new statistics display, and adjusting the visual presentation. The changes include adding new <div> elements to show learner counts, exercises, concepts, and questions, along with SVG icons. The Rust logo replaces the previous LanguageLogo component, and some existing elements have been removed or commented out. Additionally, a new section for Rust concepts has been added to the track template.

Changes

File Change Summary
app/components/track-page/header.hbs - Restructured HTML layout
- Added new flex container
- Introduced statistics display with icons
- Replaced LanguageLogo with Rust logo
- Adjusted margin classes
app/templates/track.hbs - Removed horizontal divider element
- Added new section for "Rust Primer" with TrackConceptItem components
tests/acceptance/track-page/view-track-test.js - Commented out multiple test cases
- Modified track identifier from 'go' to 'rust'
- Disabled percySnapshot assertions
app/components/track-concept-item.hbs - Introduced <LinkTo> component for navigation
- Added completion indicators and reading time display
app/components/track-concept-item.ts - Added TrackConceptItemComponent
- Defined Signature interface and module augmentation for type-checking

Possibly related PRs

  • Dark mode border improvements #2023: The changes in the main PR involve a significant reorganization of the header's content and layout, which may relate to the dark mode enhancements introduced in this PR, particularly in how components are visually presented.
  • course dark mode #2024: This PR focuses on implementing dark mode features, which are relevant to the changes in the main PR that also involve visual layout adjustments in the header.
  • Add dark mode toggle #2039: The addition of a dark mode toggle in this PR directly relates to the changes in the main PR, which also involves restructuring elements to improve user interaction and visual presentation.

Poem

🐰 A Rabbit's Ode to Track Page Redesign 🚀
Flex and stats, a new design takes flight,
Rust's logo gleaming, oh so bright!
Buttons fade, but numbers rise,
A header dancing before our eyes,
Code evolves, with rabbit's might! 🔧

✨ Finishing Touches
  • 📝 Generate Docstrings (Beta)

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

codecov bot commented Jan 29, 2025

Bundle Report

Changes will increase total bundle size by 4.22kB (0.01%) ⬆️. This is within the configured threshold ✅

Detailed changes
Bundle name Size Change
client-array-push 36.1MB 4.22kB (0.01%) ⬆️

Affected Assets, Files, and Routes:

view changes for bundle: client-array-push

Assets Changed:

Asset Name Size Change Total Size Change (%)
assets/chunk.*.js 5 bytes 62.46kB 0.01%
assets/chunk.*.js 1 bytes 12.04kB 0.01%
assets/chunk.*.js -1 bytes 53.01kB -0.0%
assets/chunk.*.js -143 bytes 30.86kB -0.46%
assets/chunk.*.js 293 bytes 31.0kB 0.95%
assets/chunk.*.js 1 bytes 22.9kB 0.0%
assets/chunk.*.js 5 bytes 107.82kB 0.0%
assets/chunk.*.js 13 bytes 160.41kB 0.01%
assets/chunk.*.js 8 bytes 404.68kB 0.0%
assets/chunk.*.js 4.04kB 2.44MB 0.17%
assets/chunk.*.js -1 bytes 234.06kB -0.0%

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e66e01d and a41871b.

📒 Files selected for processing (3)
  • app/components/track-page/header.hbs (1 hunks)
  • app/templates/track.hbs (0 hunks)
  • tests/acceptance/track-page/view-track-test.js (1 hunks)
💤 Files with no reviewable changes (1)
  • app/templates/track.hbs
🧰 Additional context used
🪛 ESLint
tests/acceptance/track-page/view-track-test.js

[error] 18-18: Do not commit pauseTest()

(ember/no-pause-test)


[error] 21-22: Delete

(prettier/prettier)

⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: test
🔇 Additional comments (1)
app/components/track-page/header.hbs (1)

1-8: LGTM! Clean layout structure with proper spacing.

The flex layout with justify-between provides good alignment between content and logo, while maintaining proper spacing for the title and description.

Comment on lines +10 to +44
<div class="flex items-center gap-x-3">
<div class="flex items-center">
<div class="flex -space-x-1 hover:space-x-1 items-center">
{{#each this.topParticipants as |user|}}
<TrackPage::Header::TopParticipantAvatar @user={{user}} />
{{/each}}
</div>

<span class="text-xs text-gray-600 dark:text-gray-400 ml-2">Join the best</span>
{{svg-jar "user-group" class="w-4 mr-1 fill-current text-gray-400 dark:text-gray-600"}}
<span class="text-xs text-gray-500">203,192 learners</span>
</div>
<div class="h-3 w-px bg-gray-200"></div>
<div class="flex items-center">
{{svg-jar "terminal" class="w-4 mr-1 fill-current text-gray-400 dark:text-gray-600"}}
<span class="text-xs text-gray-500">248 exercises</span>
</div>
{{/if}}
<div class="h-3 w-px bg-gray-200"></div>
<div class="flex items-center">
{{svg-jar "academic-cap" class="w-4 mr-1 fill-current text-gray-400 dark:text-gray-600"}}
<span class="text-xs text-gray-500">56 concepts</span>
</div>
<div class="h-3 w-px bg-gray-200"></div>
<div class="flex items-center">
{{svg-jar "clipboard-check" class="w-4 mr-1 fill-current text-gray-400 dark:text-gray-600"}}
<span class="text-xs text-gray-500">148 questions</span>
</div>
{{!-- <div class="h-1 w-1 bg-gray-400 rounded-full"></div>
<div class="flex items-center flex-wrap-reverse gap-x-2 gap-y-4">
{{#if (gt this.topParticipants.length 0)}}
<div class="flex items-center">
<div class="flex -space-x-1 hover:space-x-1 items-center">
{{#each this.topParticipants as |user|}}
<TrackPage::Header::TopParticipantAvatar @user={{user}} />
{{/each}}
</div>

<span class="text-xs text-gray-600 dark:text-gray-400 ml-2">Join the best</span>
</div>
{{/if}}
</div> --}}
</div>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Replace hardcoded statistics with dynamic values.

The statistics section looks great, but the numbers appear to be hardcoded:

  • "203,192 learners"
  • "248 exercises"
  • "56 concepts"
  • "148 questions"

These should be dynamic values from the backend to ensure accuracy.

Would you like me to help create a new component or modify this template to accept these statistics as parameters?

Comment on lines +47 to +49
<div class="ml-4 hidden md:block bg-white rounded-full p-2 border-2 border-gray-200 shadow">
<img src="https://raw.githubusercontent.com/rust-lang/rust-artwork/refs/heads/master/logo/rust-logo-blk.svg" class="h-36" />
{{!-- <LanguageLogo @language={{@language}} @variant="teal" class="h-36 dark:opacity-90" /> --}}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Restore dynamic logo handling.

The hardcoded Rust logo URL makes this component track-specific, whereas the commented-out LanguageLogo component was more flexible. Consider:

  1. Restoring the LanguageLogo component for dynamic track logos
  2. If using direct URLs, pass them as parameters instead of hardcoding
-    <div class="ml-4 hidden md:block bg-white rounded-full p-2 border-2 border-gray-200 shadow">
-      <img src="https://raw.githubusercontent.com/rust-lang/rust-artwork/refs/heads/master/logo/rust-logo-blk.svg" class="h-36" />
-      {{!-- <LanguageLogo @language={{@language}} @variant="teal" class="h-36 dark:opacity-90" /> --}}
+    <div class="ml-4 hidden md:block bg-white rounded-full p-2 border-2 border-gray-200 shadow">
+      <LanguageLogo @language={{@language}} @variant="teal" class="h-36 dark:opacity-90" />
     </div>
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<div class="ml-4 hidden md:block bg-white rounded-full p-2 border-2 border-gray-200 shadow">
<img src="https://raw.githubusercontent.com/rust-lang/rust-artwork/refs/heads/master/logo/rust-logo-blk.svg" class="h-36" />
{{!-- <LanguageLogo @language={{@language}} @variant="teal" class="h-36 dark:opacity-90" /> --}}
<div class="ml-4 hidden md:block bg-white rounded-full p-2 border-2 border-gray-200 shadow">
<LanguageLogo @language={{@language}} @variant="teal" class="h-36 dark:opacity-90" />
</div>

Comment on lines +30 to +110
// signIn(this.owner, this.server);
// createTrackLeaderboardEntries(this.server, 'go', 'redis');

await visit('/tracks/go');
assert.strictEqual(1, 1); // dummy assertion
// await visit('/tracks/go');
// assert.strictEqual(1, 1); // dummy assertion

await percySnapshot('Track - Not Started');
});
// await percySnapshot('Track - Not Started');
// });

test('it renders for logged-in user who has started course', async function (assert) {
testScenario(this.server);
signIn(this.owner, this.server);
createTrackLeaderboardEntries(this.server, 'go', 'redis');
// test('it renders for logged-in user who has started course', async function (assert) {
// testScenario(this.server);
// signIn(this.owner, this.server);
// createTrackLeaderboardEntries(this.server, 'go', 'redis');

let currentUser = this.server.schema.users.first();
let go = this.server.schema.languages.findBy({ slug: 'go' });
let redis = this.server.schema.courses.findBy({ slug: 'redis' });
// let currentUser = this.server.schema.users.first();
// let go = this.server.schema.languages.findBy({ slug: 'go' });
// let redis = this.server.schema.courses.findBy({ slug: 'redis' });

this.server.create('repository', 'withFirstStageCompleted', {
course: redis,
language: go,
user: currentUser,
});
// this.server.create('repository', 'withFirstStageCompleted', {
// course: redis,
// language: go,
// user: currentUser,
// });

await visit('/tracks/go');
assert.strictEqual(1, 1);
// await visit('/tracks/go');
// assert.strictEqual(1, 1);

await percySnapshot('Track - Started');
});
// await percySnapshot('Track - Started');
// });

test('it renders for logged-in user who has finished one course', async function (assert) {
testScenario(this.server, ['dummy', 'sqlite']);
signIn(this.owner, this.server);
createTrackLeaderboardEntries(this.server, 'go', 'dummy');
// test('it renders for logged-in user who has finished one course', async function (assert) {
// testScenario(this.server, ['dummy', 'sqlite']);
// signIn(this.owner, this.server);
// createTrackLeaderboardEntries(this.server, 'go', 'dummy');

let currentUser = this.server.schema.users.first();
let go = this.server.schema.languages.findBy({ slug: 'go' });
let dummy = this.server.schema.courses.findBy({ slug: 'dummy' });
// let currentUser = this.server.schema.users.first();
// let go = this.server.schema.languages.findBy({ slug: 'go' });
// let dummy = this.server.schema.courses.findBy({ slug: 'dummy' });

this.server.create('repository', 'withAllStagesCompleted', {
course: dummy,
language: go,
user: currentUser,
});
// this.server.create('repository', 'withAllStagesCompleted', {
// course: dummy,
// language: go,
// user: currentUser,
// });

await visit('/tracks/go');
assert.strictEqual(1, 1);
// await visit('/tracks/go');
// assert.strictEqual(1, 1);

await percySnapshot('Track - 1 Course Finished');
});
// await percySnapshot('Track - 1 Course Finished');
// });

test('it excludes alpha courses', async function (assert) {
testScenario(this.server);
signIn(this.owner, this.server);
// test('it excludes alpha courses', async function (assert) {
// testScenario(this.server);
// signIn(this.owner, this.server);

await trackPage.visit({ track_slug: 'javascript' });
assert.notOk(trackPage.cards.mapBy('title').includes('Build your own React'));
});
// await trackPage.visit({ track_slug: 'javascript' });
// assert.notOk(trackPage.cards.mapBy('title').includes('Build your own React'));
// });
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Restore critical test coverage.

A significant number of important test cases have been commented out, including:

  • Track-specific description rendering
  • Logged-in user states
  • Course completion scenarios
  • Alpha course handling

These tests cover critical user scenarios and edge cases. Instead of removing them, they should be updated to match the new header layout.

Would you like me to help update these test cases to work with the new header implementation?

Comment on lines +15 to +22
createTrackLeaderboardEntries(this.server, 'rust', 'redis');

await visit('/tracks/go');
assert.strictEqual(1, 1); // dummy assertion
await visit('/tracks/rust');
await this.pauseTest();
// assert.strictEqual(1, 1); // dummy assertion

await percySnapshot('Track - Anonymous User');

await visit('/tracks/haskell');
assert.strictEqual(1, 1); // dummy assertion
// await percySnapshot('Track - Anonymous User');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Remove debugging code and restore test assertions.

Several issues in the test setup:

  1. pauseTest() should not be committed
  2. Actual assertions are missing (only dummy assertion)
  3. Visual regression testing (Percy snapshot) is disabled
-    createTrackLeaderboardEntries(this.server, 'rust', 'redis');
-    await visit('/tracks/rust');
-    await this.pauseTest();
-    // assert.strictEqual(1, 1); // dummy assertion
-    // await percySnapshot('Track - Anonymous User');
+    createTrackLeaderboardEntries(this.server, 'rust', 'redis');
+    await visit('/tracks/rust');
+    
+    assert.strictEqual(trackPage.header.descriptionText, "Rust mastery exercises. Become your team's resident Rust expert.");
+    assert.true(trackPage.header.isVisible, "Header should be visible");
+    
+    await percySnapshot('Track - Anonymous User');
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
createTrackLeaderboardEntries(this.server, 'rust', 'redis');
await visit('/tracks/go');
assert.strictEqual(1, 1); // dummy assertion
await visit('/tracks/rust');
await this.pauseTest();
// assert.strictEqual(1, 1); // dummy assertion
await percySnapshot('Track - Anonymous User');
await visit('/tracks/haskell');
assert.strictEqual(1, 1); // dummy assertion
// await percySnapshot('Track - Anonymous User');
createTrackLeaderboardEntries(this.server, 'rust', 'redis');
await visit('/tracks/rust');
assert.strictEqual(trackPage.header.descriptionText, "Rust mastery exercises. Become your team's resident Rust expert.");
assert.true(trackPage.header.isVisible, "Header should be visible");
await percySnapshot('Track - Anonymous User');
🧰 Tools
🪛 ESLint

[error] 18-18: Do not commit pauseTest()

(ember/no-pause-test)


[error] 21-22: Delete

(prettier/prettier)

Comment on lines +112 to +130
// test('it does not show a challenge if it is deprecated', async function (assert) {
// testScenario(this.server);
// signIn(this.owner, this.server);
// createTrackLeaderboardEntries(this.server, 'go', 'redis');

// let currentUser = this.server.schema.users.first();
// let go = this.server.schema.languages.findBy({ slug: 'go' });
// let docker = this.server.schema.courses.findBy({ slug: 'docker' });
// docker.update({ releaseStatus: 'deprecated' });

// this.server.create('repository', 'withFirstStageCompleted', {
// course: docker,
// language: go,
// user: currentUser,
// });

// await visit('/tracks/go');
// assert.notOk(trackPage.cards.mapBy('title').includes('Build your own Docker'));
// });
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Restore deprecated course visibility test.

The test for ensuring deprecated courses are not displayed is important for maintaining correct course visibility rules. This regression test should be updated rather than removed to prevent accidental display of deprecated courses.

-  // test('it does not show a challenge if it is deprecated', async function (assert) {
+  test('it does not show a challenge if it is deprecated', async function (assert) {
     testScenario(this.server);
     signIn(this.owner, this.server);
     createTrackLeaderboardEntries(this.server, 'go', 'redis');
     
     // ... rest of the test setup ...
     
     await visit('/tracks/go');
-    // assert.notOk(trackPage.cards.mapBy('title').includes('Build your own Docker'));
+    assert.notOk(trackPage.cards.mapBy('title').includes('Build your own Docker'), 'Deprecated course should not be visible');
   });
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// test('it does not show a challenge if it is deprecated', async function (assert) {
// testScenario(this.server);
// signIn(this.owner, this.server);
// createTrackLeaderboardEntries(this.server, 'go', 'redis');
// let currentUser = this.server.schema.users.first();
// let go = this.server.schema.languages.findBy({ slug: 'go' });
// let docker = this.server.schema.courses.findBy({ slug: 'docker' });
// docker.update({ releaseStatus: 'deprecated' });
// this.server.create('repository', 'withFirstStageCompleted', {
// course: docker,
// language: go,
// user: currentUser,
// });
// await visit('/tracks/go');
// assert.notOk(trackPage.cards.mapBy('title').includes('Build your own Docker'));
// });
test('it does not show a challenge if it is deprecated', async function (assert) {
testScenario(this.server);
signIn(this.owner, this.server);
createTrackLeaderboardEntries(this.server, 'go', 'redis');
let currentUser = this.server.schema.users.first();
let go = this.server.schema.languages.findBy({ slug: 'go' });
let docker = this.server.schema.courses.findBy({ slug: 'docker' });
docker.update({ releaseStatus: 'deprecated' });
this.server.create('repository', 'withFirstStageCompleted', {
course: docker,
language: go,
user: currentUser,
});
await visit('/tracks/go');
assert.notOk(trackPage.cards.mapBy('title').includes('Build your own Docker'), 'Deprecated course should not be visible');
});

Copy link

github-actions bot commented Jan 29, 2025

Test Results

1 files  ±  0  1 errors  0 suites   - 1   0s ⏱️ - 7m 29s
0 tests  - 602  0 ✅  - 561  0 💤  - 41  0 ❌ ±0 
0 runs   - 617  0 ✅  - 576  0 💤  - 41  0 ❌ ±0 

For more details on these parsing errors, see this check.

Results for commit 39f5200. ± Comparison against base commit e66e01d.

♻️ This comment has been updated with latest results.

Introduce a new Rust Primer section in the track template, 
providing interactive guides for beginners. Add the 
TrackConceptItem component to display various Rust concepts 
along with their estimated reading times. Remove the unused 
academic cap SVG icon to clean up assets.
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

🧹 Nitpick comments (4)
app/components/track-concept-item.hbs (2)

8-17: Enhance accessibility for concept items.

The component lacks proper ARIA attributes for better screen reader support.

Apply this diff to improve accessibility:

 <div class="flex items-center">
+  <div role="status" aria-label="{{if @isComplete 'Completed' 'Not completed'}}"
   class="w-2 h-2 {{if @isComplete 'bg-teal-500' 'bg-gray-200 dark:bg-gray-700'}} rounded-full mr-2"></div>

   {{! TODO: Qualify all heroicons with solid/outline }}
   {{svg-jar "academic-cap-outline" class="w-6 mr-1.5 text-gray-400 dark:text-gray-600"}}

-  <div class="prose dark:prose-invert prose-sm">
+  <div class="prose dark:prose-invert prose-sm" aria-label="Concept name">
     {{@name}}
   </div>
 </div>

23-31: Remove commented out code for questions feature.

The commented section suggests a planned feature for displaying questions. Either implement the feature or remove the commented code.

Would you like me to help implement the questions feature or create an issue to track this enhancement?

app/templates/track.hbs (2)

34-38: Fix typo in conclusion text.

There's a typo in the conclusion text: "go" should be "got".

Apply this diff to fix the typo:

-            Once you've go the basics down, time to move on to real projects.
+            Once you've got the basics down, time to move on to real projects.

19-32: Consider making reading times configurable.

Reading times are currently hardcoded in the template. Consider moving these to a configuration file or data model for easier maintenance.

Example configuration structure:

interface ConceptConfig {
  name: string;
  estimatedReadingTimeInMinutes: number;
  order: number;
}

const RUST_CONCEPTS: ConceptConfig[] = [
  { name: "Defining variables", estimatedReadingTimeInMinutes: 2, order: 1 },
  // ... other concepts
];

This would allow for easier updates and maintenance of the concept list.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a41871b and 39f5200.

⛔ Files ignored due to path filters (1)
  • public/assets/images/heroicons/outline/academic-cap-outline.svg is excluded by !**/*.svg
📒 Files selected for processing (3)
  • app/components/track-concept-item.hbs (1 hunks)
  • app/components/track-concept-item.ts (1 hunks)
  • app/templates/track.hbs (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: test
🔇 Additional comments (1)
app/components/track-concept-item.hbs (1)

1-6: Add missing route parameters for catalog navigation.

The LinkTo component points to the "catalog" route but doesn't specify required parameters. This could lead to navigation issues.

Run this script to check the route definition:

Comment on lines +3 to +11
interface Signature {
Element: HTMLButtonElement;

Args: {
name: string;
isComplete: boolean;
estimatedReadingTimeInMinutes: number;
};
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix element type in component signature.

The interface specifies HTMLButtonElement but the component template uses <LinkTo> which renders as an anchor element.

Apply this diff to fix the element type:

 interface Signature {
-  Element: HTMLButtonElement;
+  Element: HTMLAnchorElement;

   Args: {
     name: string;
     isComplete: boolean;
     estimatedReadingTimeInMinutes: number;
   };
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
interface Signature {
Element: HTMLButtonElement;
Args: {
name: string;
isComplete: boolean;
estimatedReadingTimeInMinutes: number;
};
}
interface Signature {
Element: HTMLAnchorElement;
Args: {
name: string;
isComplete: boolean;
estimatedReadingTimeInMinutes: number;
};
}

Comment on lines +11 to +12
{{! TODO: Qualify all heroicons with solid/outline }}
{{svg-jar "academic-cap-outline" class="w-6 mr-1.5 text-gray-400 dark:text-gray-600"}}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Standardize heroicon usage across the codebase

The codebase shows inconsistent usage of the academic-cap icon:

  • app/components/track-concept-item.hbs: Uses academic-cap-outline
  • app/components/course-card.hbs: Uses academic-cap
  • app/components/tracks-page/track-card.hbs: Uses academic-cap
  • app/components/track-page/header.hbs: Uses academic-cap
🔗 Analysis chain

Address TODO comment about heroicons qualification.

The comment indicates a need to qualify heroicons with solid/outline variants.

Run this script to check heroicon usage patterns:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check heroicon usage patterns in the codebase
rg --type hbs "svg-jar.*heroicon" app/

Length of output: 40


Script:

#!/bin/bash
# Search for all svg-jar usages
echo "=== All svg-jar usages ==="
rg --type hbs "{{svg-jar" -A 1

echo -e "\n=== Files containing svg-jar ==="
fd --type f --extension hbs --exec grep -l "svg-jar" {}

echo -e "\n=== Looking for academic-cap specifically ==="
rg --type hbs "academic-cap"

echo -e "\n=== Checking for icon documentation ==="
fd "ICON|icon" --type f --extension md

Length of output: 10864

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant