Skip to content

Conversation

@Mudaafi
Copy link
Contributor

@Mudaafi Mudaafi commented Apr 8, 2025

Fix Description

Fixes a bug where two inputs were rendered with the same id for Sort Options. While the Desktop sort options are set to display: none, we are still rendering the elements to the DOM. This leads to weird behaviors on mobile due to invalid DOM structure.

To fix this, rather than attempting to use JavaScript to determine when the css screen breakpoints are reached, we keep the existing behavior, but just render the inputs with different ids.

Pull Request Checklist

Before you submit a pull request, please make sure you have to following:

  • I have added or updated TypeScript types for my changes, ensuring they are compatible with the existing codebase.
  • I have added JSDoc comments to my TypeScript definitions for improved documentation.
  • I have added tests that prove my fix is effective or that my feature works.
  • I have added any necessary documentation (if appropriate).
  • I have made sure my PR is up-to-date with the main branch.

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no API changes)
  • Documentation content changes
  • TypeScript type definitions update
  • Other... Please describe:

@Mudaafi Mudaafi requested a review from lordvkrum April 8, 2025 00:26
@Mudaafi Mudaafi requested a review from a team as a code owner April 8, 2025 00:26
Copy link

@jjl014 jjl014 left a comment

Choose a reason for hiding this comment

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

The changes look good to me! 👍

@Mudaafi Mudaafi merged commit e53c2e1 into main Apr 8, 2025
10 of 11 checks passed
@Mudaafi Mudaafi deleted the ci-4339-os-plp-ui-fix-sort-component-rendering-to-input-label-pairs branch April 8, 2025 00:54
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