Skip to content

Design preview for code example voting #2810

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.

Sign up for GitHub

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 1 commit into
base: main
Choose a base branch
from
+111 −3
Open

Design preview for code example voting #2810

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion app/components/course-page/course-stage-step/community-solution-card/content.hbs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,4 +84,6 @@
</div>
</:header>
</FileContentsCard>
{{/each}}
{{/each}}

Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

The feedback section component is missing solution data

The feedback section component is added correctly, but it doesn't receive any data about the current solution. For the design preview this might be intentional, but for actual functionality, it should receive the solution data to record votes.

-<CoursePage::CourseStageStep::CommunitySolutionCard::FeedbackSection class="pt-6 pb-3" />
+<CoursePage::CourseStageStep::CommunitySolutionCard::FeedbackSection 
+  @solution={{@solution}} 
+  @metadata={{@metadataForUpvote}} 
+  class="pt-6 pb-3" 
+/>
📝 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
<CoursePage::CourseStageStep::CommunitySolutionCard::FeedbackSection class="pt-6 pb-3" />
<CoursePage::CourseStageStep::CommunitySolutionCard::FeedbackSection
@solution={{@solution}}
@metadata={{@metadataForUpvote}}
class="pt-6 pb-3"
/>

<CoursePage::CourseStageStep::CommunitySolutionCard::FeedbackSection class="pt-6 pb-3" />
46 changes: 46 additions & 0 deletions app/components/course-page/course-stage-step/community-solution-card/feedback-section.hbs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
<div class="flex flex-col items-center gap-2" ...attributes>
<AnimatedContainer class="w-full">
<div class="text-sm text-gray-700 dark:text-gray-300 text-center w-full">
{{#animated-if this.tempHasVotedRecently use=this.transition duration=200}}
<p class="text-teal-500 font-medium">
Thanks for your feedback!
</p>
{{else}}
<p>
Was this example helpful?
</p>
{{/animated-if}}
</div>
</AnimatedContainer>

<div class="flex items-center gap-1.5 flex-wrap">
<TertiaryButton
{{on "click" this.handleUpvoteButtonClick}}
class={{if this.tempHasDownvoted "opacity-50 hover:opacity-100 transition-opacity duration-200"}}
>
<div class="flex items-center gap-1">
{{svg-jar "thumb-up" class=(concat "w-5 " (if this.tempHasUpvoted "text-teal-500" "text-gray-400 dark:text-gray-600"))}}

<span class={{if this.tempHasUpvoted "text-teal-500" "text-gray-500"}}>
Helpful
</span>
</div>

<EmberTooltip @text="Your feedback helps us surface better examples." @side="bottom" @delay={{250}} />
</TertiaryButton>
<TertiaryButton
{{on "click" this.handleDownvoteButtonClick}}
class={{if this.tempHasUpvoted "opacity-50 hover:opacity-100 transition-opacity duration-200"}}
>
<div class="flex items-center gap-1">
{{svg-jar "thumb-down" class=(concat "w-5 " (if this.tempHasDownvoted "text-red-600" "text-gray-400 dark:text-gray-600"))}}

<span class={{if this.tempHasDownvoted "text-red-600" "text-gray-500"}}>
Not helpful
</span>
</div>

<EmberTooltip @text="Your feedback helps us identify examples that need review." @side="bottom" @delay={{250}} />
</TertiaryButton>
</div>
</div>
59 changes: 59 additions & 0 deletions app/components/course-page/course-stage-step/community-solution-card/feedback-section.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
import Component from '@glimmer/component';
import { tracked } from '@glimmer/tracking';
import { task, timeout } from 'ember-concurrency';
import fade from 'ember-animated/transitions/fade';
import { action } from '@ember/object';

interface Signature {
Element: HTMLDivElement;
}

export default class CommunitySolutionCardFeedbackSectionComponent extends Component<Signature> {
transition = fade;

@tracked tempHasVotedRecently = false;
@tracked tempHasUpvoted = false;
@tracked tempHasDownvoted = false;

@action
handleDownvoteButtonClick() {
// No feedback if the user is "reversing" their vote
if (this.tempHasDownvoted) {
this.tempHasDownvoted = false;
this.tempHasVotedRecently = false;

return;
}

this.flashSuccessMessageTask.perform();
this.tempHasUpvoted = false;
this.tempHasDownvoted = true;
}

@action
handleUpvoteButtonClick() {
// No feedback if the user is "reversing" their vote
if (this.tempHasUpvoted) {
this.tempHasUpvoted = false;
this.tempHasVotedRecently = false;

return;
}

this.flashSuccessMessageTask.perform();
this.tempHasDownvoted = false;
this.tempHasUpvoted = true;
}

flashSuccessMessageTask = task({ keepLatest: true }, async () => {
this.tempHasVotedRecently = true;
await timeout(1500);
this.tempHasVotedRecently = false;
});
Comment on lines +14 to +52
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Missing test coverage for component logic

The static analysis indicates that much of the component logic is not covered by tests. Consider adding tests for:

  1. The upvote/downvote toggle functionality
  2. The success message flash behavior
  3. Edge cases like multiple rapid clicks

You can verify the current test coverage with:

🏁 Script executed:

#!/bin/bash
# Check current test coverage for feedback-section component
find tests -type f -name "*feedback-section*" -o -name "*community-solution*" | xargs cat

Length of output: 16787

Add targeted tests for flash message and rapid-click behavior

While the existing acceptance tests cover the basic upvote/downvote toggling via UI, we still need to add focused tests for the component’s internal logic:

• Test the flashSuccessMessageTask in isolation
– Verify that tempHasVotedRecently flips to true, then back to false after the 1.5 s timeout (e.g. using await settled() or ember-concurrency’s test helpers).
• Test rapid‐click edge cases
– Ensure that clicking upvote/downvote multiple times in quick succession doesn’t trigger extra flashes or leave the component in an inconsistent state.
• (If not already covered) Add a small unit/integration test for the toggle logic directly on the component, asserting initial state and flip behavior of tempHasUpvoted/tempHasDownvoted.

You can place these in a new integration test under:
  tests/integration/components/course-page/course-stage-step/community-solution-card/feedback-section-test.js

🧰 Tools
🪛 GitHub Check: codecov/patch

[warning] 23-24: app/components/course-page/course-stage-step/community-solution-card/feedback-section.ts#L23-L24
Added lines #L23 - L24 were not covered by tests

[warning] 26-26: app/components/course-page/course-stage-step/community-solution-card/feedback-section.ts#L26
Added line #L26 was not covered by tests

[warning] 29-31: app/components/course-page/course-stage-step/community-solution-card/feedback-section.ts#L29-L31
Added lines #L29 - L31 were not covered by tests

[warning] 38-39: app/components/course-page/course-stage-step/community-solution-card/feedback-section.ts#L38-L39
Added lines #L38 - L39 were not covered by tests

[warning] 41-41: app/components/course-page/course-stage-step/community-solution-card/feedback-section.ts#L41
Added line #L41 was not covered by tests

[warning] 44-46: app/components/course-page/course-stage-step/community-solution-card/feedback-section.ts#L44-L46
Added lines #L44 - L46 were not covered by tests

[warning] 50-52: app/components/course-page/course-stage-step/community-solution-card/feedback-section.ts#L50-L52
Added lines #L50 - L52 were not covered by tests

}

declare module '@glint/environment-ember-loose/registry' {
export default interface Registry {
'CoursePage::CourseStageStep::CommunitySolutionCard::FeedbackSection': typeof CommunitySolutionCardFeedbackSectionComponent;
}
}
Comment on lines +11 to +59
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Missing actual vote persistence functionality

The component currently only manages temporary visual state (note the temp prefixes in variable names) but doesn't persist votes or make API calls. For a design preview this is fine, but for production functionality, it should:

  1. Accept @Solution as an argument to know which solution is being voted on
  2. Make API calls to record votes
  3. Handle loading and error states

Here's a suggested enhancement to the component interface:

interface Signature {
  Element: HTMLDivElement;
+  Args: {
+    solution: SolutionModel;
+    metadata?: Record<string, unknown>;
+  };
}

Additionally, implement actual API calls in the handlers:

@action
async handleUpvoteButtonClick() {
  // Existing logic for UI state...
  
  try {
    await this.args.solution.upvote(this.args.metadata);
    // Success handling
  } catch (error) {
    // Error handling
    this.tempHasUpvoted = false;
    this.tempHasVotedRecently = false;
  }
}
🧰 Tools
🪛 GitHub Check: codecov/patch

[warning] 23-24: app/components/course-page/course-stage-step/community-solution-card/feedback-section.ts#L23-L24
Added lines #L23 - L24 were not covered by tests

[warning] 26-26: app/components/course-page/course-stage-step/community-solution-card/feedback-section.ts#L26
Added line #L26 was not covered by tests

[warning] 29-31: app/components/course-page/course-stage-step/community-solution-card/feedback-section.ts#L29-L31
Added lines #L29 - L31 were not covered by tests

[warning] 38-39: app/components/course-page/course-stage-step/community-solution-card/feedback-section.ts#L38-L39
Added lines #L38 - L39 were not covered by tests

[warning] 41-41: app/components/course-page/course-stage-step/community-solution-card/feedback-section.ts#L41
Added line #L41 was not covered by tests

[warning] 44-46: app/components/course-page/course-stage-step/community-solution-card/feedback-section.ts#L44-L46
Added lines #L44 - L46 were not covered by tests

[warning] 50-52: app/components/course-page/course-stage-step/community-solution-card/feedback-section.ts#L50-L52
Added lines #L50 - L52 were not covered by tests

5 changes: 3 additions & 2 deletions app/components/course-page/course-stage-step/community-solution-card/header.hbs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,12 @@
</div>

<div class="flex items-center shrink-0">
{{#unless (eq @solution.user this.authenticator.currentUser)}}
{{! Remove, replace with buttons at the bottom of the card }}
{{!-- {{#unless (eq @solution.user this.authenticator.currentUser)}}
<CoursePage::CourseStageStep::CommunitySolutionCard::UpvoteButton @solution={{@solution}} @metadata={{@metadataForUpvote}} class="mr-2" />
<CoursePage::CourseStageStep::CommunitySolutionCard::DownvoteButton @solution={{@solution}} @metadata={{@metadataForDownvote}} />
<div class="w-px h-3 bg-gray-200 dark:bg-white/5 mx-2"></div>
{{/unless}}
{{/unless}} --}}
Comment on lines +23 to +28
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

Missing user restriction in new feedback component

The commented-out code shows the previous implementation had a condition to only show vote buttons if the solution wasn't created by the current user. This same logic should be applied to the new feedback section component to maintain consistent behavior.

This restriction logic should either be:

  1. Implemented in the feedback-section component itself (by passing the current user)
  2. Added as a condition when rendering the component in content.hbs
// In content.hbs:
+{{#unless (eq @solution.user this.authenticator.currentUser)}}
  <CoursePage::CourseStageStep::CommunitySolutionCard::FeedbackSection @solution={{@solution}} class="pt-6 pb-3" />
+{{/unless}}
📝 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
{{! Remove, replace with buttons at the bottom of the card }}
{{!-- {{#unless (eq @solution.user this.authenticator.currentUser)}}
<CoursePage::CourseStageStep::CommunitySolutionCard::UpvoteButton @solution={{@solution}} @metadata={{@metadataForUpvote}} class="mr-2" />
<CoursePage::CourseStageStep::CommunitySolutionCard::DownvoteButton @solution={{@solution}} @metadata={{@metadataForDownvote}} />
<div class="w-px h-3 bg-gray-200 dark:bg-white/5 mx-2"></div>
{{/unless}}
{{/unless}} --}}
{{!-- Only show feedback section if the solution wasn't created by the current user --}}
{{#unless (eq @solution.user this.authenticator.currentUser)}}
<CoursePage::CourseStageStep::CommunitySolutionCard::FeedbackSection
@solution={{@solution}}
class="pt-6 pb-3"
/>
{{/unless}}


{{#if (gt @solution.approvedCommentsCount 0)}}
<div class="flex items-center">
Expand Down
Loading