Skip to content

Conversation

@RoyEJohnson
Copy link
Contributor

@RoyEJohnson RoyEJohnson commented May 7, 2025

@RoyEJohnson RoyEJohnson requested a review from mwvolo May 7, 2025 22:50
@RoyEJohnson
Copy link
Contributor Author

This also fixes the logout button

Making a separate edit button for it is tricky due to the way the "editable" component works.
and restyle Add an email button
@RoyEJohnson RoyEJohnson force-pushed the core-941-update-visual-presentation-of-items-on-my-profile branch from 18fcd06 to da73ac2 Compare May 9, 2025 15:49
Copy link
Member

@mwvolo mwvolo left a comment

Choose a reason for hiding this comment

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

Looks good to me. It's also on dev if you'd like to take a peek before merging
https://dev.accounts.openstax.org/i/profile

@mwvolo mwvolo requested a review from Copilot May 12, 2025 16:24
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR updates the visual presentation of profile items by adjusting HTML markup, styles, and the associated JavaScript behavior to support a more accessible and modern UI. Key changes include:

  • Replacing button elements and adjusting markup in the profile view for the name and email sections.
  • Updating helper methods to use semantic elements (details/summary) for collapsible content.
  • Adjusting styles and CoffeeScript callbacks to align with the new markup.

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
app/views/newflow/base/profile_newflow.html.erb Updated markup for the profile name and other sign-in options with details
app/helpers/profile_helper.rb Refactored email entry markup to use details/summary for better semantics
app/assets/stylesheets/profile.scss Adjusted styles for the new UI elements and updated display properties
app/assets/javascripts/profile/name.coffee Modified the callback to update nested .text-content element post-edit
app/assets/javascripts/profile/email.coffee Updated selector to target the new summary based email entry content
app/assets/javascripts/profile/authentication.coffee Removed obsolete jQuery toggle logic for other sign-in options
Comments suppressed due to low confidence (2)

app/assets/stylesheets/profile.scss:61

  • [nitpick] Consider removing the trailing semicolon after the .glyphicon block for consistency with SCSS syntax conventions.
.glyphicon { margin-left: 2rem; };

app/views/newflow/base/profile_newflow.html.erb:130

  • Using method: :get for the sign-out action deviates from RESTful conventions (DELETE is typically used). Confirm that this change is intentional.
method: :get,

)
%>
<% end %>
<div>
Copy link

Copilot AI May 12, 2025

Choose a reason for hiding this comment

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

Unexpected opening

inside the details element; it seems intended to close the previously opened 'controls' div. Please revise the markup to ensure proper element closure.

Suggested change
<div>
</div>

Copilot uses AI. Check for mistakes.
@mwvolo mwvolo merged commit 559c4e1 into main May 12, 2025
6 checks passed
@mwvolo mwvolo deleted the core-941-update-visual-presentation-of-items-on-my-profile branch May 12, 2025 18:43
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.

3 participants