Skip to content
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

Jump to bottom

Don't show Continue/Back if the concept has been completed #2738

Merged
merged 3 commits into from
Mar 28, 2025
Merged

Don't show Continue/Back if the concept has been completed #2738

merged 3 commits into from
Mar 28, 2025

Conversation

VasylMarchuk
Copy link
Collaborator

@VasylMarchuk VasylMarchuk commented Mar 16, 2025 •
edited by coderabbitai bot
Loading

Closes #2727

Brief

After completing the concept, Continue/Back buttons remain visible, and continue responding to Enter/Backspace key presses, stealing them from the Feedback component.

Details

This hides Continue/Back buttons if the concept has been completed.

Checklist

  • I've thoroughly self-reviewed my changes
  • I've added tests for my changes, unless they affect admin-only areas (or are otherwise not worth testing)
  • I've verified any visual changes using Percy (add a commit with [percy] in the message to trigger)

Summary by CodeRabbit

  • New Features

    • Introduced a new feedback dropdown component for improved user interactions.
    • Added keyboard navigation support for the feedback popup, allowing users to use Delete and Backspace keys.

  • Bug Fixes

    • Updated the workflow so that the continue/step back option is now only displayed when further action is needed, ensuring that it no longer appears after the process is finished.

  • Tests

    • Enhanced acceptance tests for feedback popup interactions and visibility assertions.
    • Added a new page object for the feedback dropdown to streamline testing.

@VasylMarchuk VasylMarchuk added the bug Something isn't working label Mar 16, 2025
@VasylMarchuk VasylMarchuk requested a review from rohitpaulk March 16, 2025 16:07
@VasylMarchuk VasylMarchuk self-assigned this Mar 16, 2025
@coderabbitai coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 16, 2025 •
edited
Loading

Walkthrough

This change introduces conditional rendering to the Concept::ContinueOrStepBack component in the concept page template, ensuring it only displays when this.hasFinished is false. Additionally, new acceptance tests for keyboard interactions in the feedback popup have been added, and existing tests were updated to check for visibility rather than open status. A new page object for the feedback dropdown component was also created, replacing inline definitions in several test files.

Changes

File Change Summary
app/components/.../index.hbs Wrapped the Concept::ContinueOrStepBack component with a {{#unless this.hasFinished}} block for conditional rendering.
tests/acceptance/concepts-test.js Added a test for using Delete and Backspace keys in the feedback popup.
tests/acceptance/submit-site-feedback-test.js Updated assertions to check visibility of the feedback dropdown instead of its open status.
tests/pages/components/feedback-dropdown.ts Created a new page object for the feedback dropdown component.
tests/pages/components/header.js Replaced inline feedbackDropdown definition with an import of the FeedbackDropdown component.
tests/pages/concept-page.js Added a new property for feedbackDropdown from the imported FeedbackDropdown.
tests/pages/course-page.js Replaced inline feedbackDropdown definition with an import of the FeedbackDropdown component.

Assessment against linked issues

Objective Addressed Explanation
Fix Del/Backspace key issue in feedback pop-up (#2727) ✅

Possibly related PRs

Suggested reviewers

  • rohitpaulk

Poem

I'm a rabbit tapping keys so light,
Hopping through code from morning to night.
I guard the keys with a conditional hop,
Ensuring no rogue clicks will ever drop.
Cheers to clean code and bugs that flee—
My whiskers wiggle in joyful glee!

📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)

📥 Commits

Reviewing files that changed from the base of the PR and between b98178f and fe6b6b8.

📒 Files selected for processing (6)
  • tests/acceptance/concepts-test.js (1 hunks)
  • tests/acceptance/submit-site-feedback-test.js (2 hunks)
  • tests/pages/components/feedback-dropdown.ts (1 hunks)
  • tests/pages/components/header.js (1 hunks)
  • tests/pages/concept-page.js (2 hunks)
  • tests/pages/course-page.js (3 hunks)
🔇 Additional comments (7)
tests/pages/components/feedback-dropdown.ts (1)

1-11: Well-structured page object for feedback dropdown testing

This new file creates a reusable page object for the feedback dropdown component, following good practices by centralizing interaction methods for testing. The implementation includes all necessary actions (clicking, filling, triggering keys) and assertions (visibility) needed for comprehensive testing of the feedback dropdown functionality.

tests/pages/concept-page.js (1)

4-4: Good modularization of feedback dropdown functionality

The import and integration of the FeedbackDropdown page object improves code organization by following a modular approach, removing potential duplication across test files and centralizing the feedback dropdown functionality.

Also applies to: 28-28

tests/pages/components/header.js (1)

1-3: Clean implementation of feedback dropdown integration

The changes properly simplify imports and incorporate the centralized FeedbackDropdown component, maintaining consistency with other files while reducing duplication.

Also applies to: 10-10

tests/acceptance/concepts-test.js (1)

922-946: Effectively tests keyboard interaction in feedback popup

This new test properly verifies that Delete and Backspace keys work correctly in the feedback popup without being intercepted by other components - addressing the issue where Continue/Back buttons were interfering with the Feedback component. The test provides good coverage of the user interaction flow.

tests/pages/course-page.js (2)

10-10: Good refactoring: Improved modularity with FeedbackDropdown component

This change effectively replaces an inline implementation of the feedback dropdown with a dedicated imported component. This improves code organization and maintainability by centralizing the feedback dropdown functionality.

Also applies to: 170-170

20-20: Proper cleanup: Removed unused imports

The removal of the fillable import is appropriate since it's no longer used after refactoring the feedback dropdown implementation.

tests/acceptance/submit-site-feedback-test.js (1)

30-30: Appropriate assertion updates for FeedbackDropdown component

The assertions have been updated to check isVisible instead of isOpen, which properly aligns with the refactored FeedbackDropdown component. This change maintains the same test logic while adapting to the new implementation that uses visibility status rather than open status to determine the dropdown state.

Also applies to: 35-35, 38-38, 47-47

🪧 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 resolve resolve all the CodeRabbit review comments.
  • @coderabbitai plan to trigger planning for file edits and PR creation.
  • @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.

@codecov Codecov
Copy link

codecov bot commented Mar 16, 2025 •
edited
Loading

Bundle Report

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

Detailed changes
Bundle name Size Change
client-array-push 36.56MB 1.76kB (0.0%) ⬆️

Affected Assets, Files, and Routes:

view changes for bundle: client-array-push

Assets Changed:

Asset Name Size Change Total Size Change (%)
assets/chunk.*.js 1.62kB 2.83MB 0.06%
assets/chunk.*.js 56 bytes 25.9kB 0.22%
assets/chunk.*.js 82 bytes 26.53kB 0.31%

@github-actions GitHub Actions
Copy link

github-actions bot commented Mar 16, 2025 •
edited
Loading

Test Results

629 tests  +629   585 ✅ +585   7m 54s ⏱️ + 7m 54s
  1 suites +  1    44 💤 + 44 
  1 files   ±  0     0 ❌ ±  0 

Results for commit fe6b6b8. ± Comparison against base commit 9db4e56.

♻️ This comment has been updated with latest results.

@codecov Codecov
Copy link

codecov bot commented Mar 16, 2025 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

✅ All tests successful. No failed tests found.

📢 Thoughts on this report? Let us know!

@rohitpaulk
Copy link
Member

@VasylMarchuk let's add some kind of test for this? Ideally one that replicates the actual user-reported bug, and not just one that checks whether the continue button is rendered or not.

@rohitpaulk
Copy link
Member

Good to merge in the meantime

@VasylMarchuk VasylMarchuk force-pushed the fix-del-backspace-on-concept branch from 69b613a to ee57189 Compare March 22, 2025 12:05
@VasylMarchuk VasylMarchuk force-pushed the fix-del-backspace-on-concept branch from ee57189 to b98178f Compare March 28, 2025 09:39
@VasylMarchuk
Copy link
Collaborator Author

Test added, merging now!

@VasylMarchuk VasylMarchuk merged commit 3459083 into main Mar 28, 2025
9 checks passed
@VasylMarchuk VasylMarchuk deleted the fix-del-backspace-on-concept branch March 28, 2025 15:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rohitpaulk rohitpaulk rohitpaulk approved these changes

Assignees

@VasylMarchuk VasylMarchuk

Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Del/Backspace key broken for feedback pop-up at the bottom of Concept page
2 participants
@VasylMarchuk @rohitpaulk