Skip to content

Conversation

@jzempel
Copy link
Member

@jzempel jzempel commented Feb 15, 2024

Description

Floating UI uses logical placement. As a result, there is less placement prop mapping to do then with Popper. This PR addresses a handful of placement combinations that were incorrect with RTL layouts.

Detail

Before with placement="top-start"

before

After with placement="top-start"

after

Checklist

  • 👌 design updates will be Garden Designer approved (add the designer as a reviewer)
  • 🌐 demo is up-to-date (npm start)
  • ⬅️ renders as expected with reversed (RTL) direction
  • 🤘 renders as expected with Bedrock CSS (?bedrock)
  • 💂‍♂️ includes new unit tests. Maintain existing coverage (always >= 96%)
  • tested for WCAG 2.1 AA accessibility compliance
  • 📝 tested in Chrome, Firefox, Safari, and Edge

@jzempel jzempel requested a review from a team as a code owner February 15, 2024 16:20
@coveralls
Copy link

coveralls commented Feb 15, 2024

Coverage Status

coverage: 96.194% (-0.04%) from 96.235%
when pulling 1225bd9 on jzempel/fix-placement
into 095d516 on main.

Copy link
Contributor

@geotrev geotrev left a comment

Choose a reason for hiding this comment

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

LGTM!

@jzempel jzempel merged commit d4dc396 into main Feb 15, 2024
@jzempel jzempel deleted the jzempel/fix-placement branch February 15, 2024 18:27
geotrev pushed a commit that referenced this pull request Feb 15, 2024
Fixes RTL placement for:
- `top-start`
- `top-end`
- `bottom-start`
- `bottom-end`
@geotrev geotrev mentioned this pull request Feb 15, 2024
7 tasks
geotrev added a commit that referenced this pull request Feb 16, 2024
changes added:
- fix(menu): correct RTL menu placement and arrow position (#1708)

Co-authored-by: Jonathan Zempel <jzempel@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

6 participants