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.

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

Feature: Add “Continue” button and badge for selected path #4513

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

Conversation

davidha99
Copy link

Because

A user might get confused about the current behavior of the "Resume" button in All Paths (#3230, #4495).

This PR

Helps to differentiate a selected path from the rest by:

  • Adding a "Selected" badge to the currently selected path.
  • Keeping only one "Continue" button instead of "Resume" and "View" buttons to selected path.

Note: User needs to be logged in.

Before (Foundations path selected)

image

image

After (Foundations path selected)

image

image

Before (Ruby on Rails path selected)

image

After (Ruby on Rails path selected)

image

Before (Javascript path selected)

image

After (Javascript path selected)

image

Issue

Closes #4495

Pull Request Requirements

  • I have thoroughly read and understand The Odin Project Contributing Guide
  • The title of this PR follows the keyword: brief description of change format, using one of the following keywords:
    • Feature - adds new or amends existing user-facing behavior
    • Chore - changes that have no user-facing value, refactors, dependency bumps, etc
    • Fix - bug fixes
  • The Because section summarizes the reason for this PR
  • The This PR section has a bullet point list describing the changes in this PR
  • I have verified all tests and linters pass after making these changes.
  • If this PR addresses an open issue, it is linked in the Issue section
  • If applicable, this PR includes new or updated automated tests

Copy link
Contributor

@JoshDevHub JoshDevHub left a comment

Choose a reason for hiding this comment

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

This is a cool change. I always found these buttons a bit confusing.

Just have a couple of small nits.

app/views/paths/index.html.erb Outdated Show resolved Hide resolved
app/views/paths/index.html.erb Outdated Show resolved Hide resolved
@JoshDevHub
Copy link
Contributor

Awesome. I'll just wait for Kevin to look things over and maybe create a review deploy for us to preview things. Thank you for contributing 🚀

@KevinMulhern KevinMulhern temporarily deployed to odin-review-app-pr-4513 April 25, 2024 14:16 Inactive
@KevinMulhern KevinMulhern temporarily deployed to odin-review-app-pr-4513 April 30, 2024 08:55 Inactive
Copy link
Member

@KevinMulhern KevinMulhern left a comment

Choose a reason for hiding this comment

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

This is looking great @davidha99 💪 thanks for doing this!

Just one small suggestion from me.

<div class="absolute top-3.5 right-3.5">
<% if current_user.present? && current_user.on_path?(path) %>
<%= render Ui::BadgeComponent.new(color: 'green') do %>
<%= @badge_text %>
Copy link
Member

Choose a reason for hiding this comment

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

Nit: We can hardcode "Selected" in the badges. I think if we needed the text to be dynamic based on some state, using an instance var would be the way to go. But the content should remain static for now.

Suggested change
<%= @badge_text %>
Selected

Copy link
Author

Choose a reason for hiding this comment

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

Hi @KevinMulhern thanks for the suggestion. I've updated the code 👍

@KevinMulhern KevinMulhern temporarily deployed to odin-review-app-pr-4513 April 30, 2024 14:23 Inactive
Copy link
Member

@KevinMulhern KevinMulhern left a comment

Choose a reason for hiding this comment

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

Great stuff! thanks @davidha99 🚀

@davidha99
Copy link
Author

Thanks! Glad to help 🙌

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

Successfully merging this pull request may close these issues.

Add “Continue” button and badge for selected path
3 participants