migrate from Popper.js to Floating-UI #4393

G07cha · 2023-11-26T20:27:51Z

Breaking changes:

popperModifiers only accepts Floating-UI specific middleware
'auto', 'auto-start', and 'auto-end' placements are no longer supported
popperProps only accept useFloating options

Breaking changes: - `popperModifiers` only accepts [Floating-UI specific middleware](https://floating-ui.com/docs/middleware) - 'auto', 'auto-start', and 'auto-end' placements are no longer supported - `popperProps` only accept [`useFloating` options](https://floating-ui.com/docs/usefloating#options) Closes Hacker0x01#3842

pullrequest

✅ This pull request was sent to the PullRequest network.

@G07cha you can click here to see the review status or cancel the code review job.

pullrequest

PullRequest Breakdown

Reviewable lines of change

+ 134
- 188

61% JavaScript
35% SCSS
2% JavaScript (tests)
2% JSON

Generated lines of change

+ 41
- 25

Type of change

Feature - These changes are adding a new feature or improvement to existing code.

pullrequest

Looks like a cleaner solution! I can see the arrow change and various code formatting/clean-up changes.

Steven S

Reviewed with ❤️ by PullRequest

pullrequest

Based on the description this appears to update the underlying rendering library as described and includes some steady changes. I see no issues to prevent this from moving forward

James D

Reviewed with ❤️ by PullRequest

martijnrusschen · 2023-11-27T08:32:07Z

docs-site/src/components/Examples/index.js

@@ -160,11 +160,11 @@ export default class exampleComponents extends React.Component {
        <div>
          Full docs for the popper can be found at{" "}


This still mentions popper

I wasn't sure about that one, as "popper" name is still used in props (popperClassname, popperPlacement, etc) and that might confuse the reader otherwise. Would it be better to mention floating UI directly? I.e. "Full docs for the Floating UI component can be found at".

martijnrusschen · 2023-11-27T08:33:54Z

src/popper_component.jsx

@@ -1,20 +1,17 @@
 import classnames from "classnames";
 import React from "react";
 import PropTypes from "prop-types";
-import { Manager, Reference, Popper } from "react-popper";
-import { placements } from "@popperjs/core/lib";
+import { Manager } from "react-popper";


Is this still being used?

Good catch, missed that one. All references to @popperjs/core or react-popper packages should be gone now.

src/stylesheets/datepicker.scss

pullrequest

PullRequest reviewed the updates made to #4393 up until the latest commit (1456591). No further issues were found.

Reviewed by:

Steven S

It was previously used to attach additonal props but that's no longer needed.

pullrequest

PullRequest reviewed the updates made to #4393 up until the latest commit (267c138). No further issues were found.

Reviewed by:

Steven S

codecov · 2023-11-28T08:36:09Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (a92d5e7) 95.32% compared to head (267c138) 95.33%.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #4393   +/-   ##
=======================================
  Coverage   95.32%   95.33%           
=======================================
  Files          28       29    +1     
  Lines        2479     2484    +5     
  Branches     1020     1019    -1     
=======================================
+ Hits         2363     2368    +5     
  Misses        116      116

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

martijnrusschen

This looks good, I'll do some more manual testing before merging and preparing a release.

pullrequest

✖️ This code review was cancelled. See Details

BREAKING CHANGE: Require date-fns 4 BREAKING CHANGE: bump react-datepicker from v4 to v7 List of breaking changes: - v5: Hacker0x01/react-datepicker#4393 - v6: Hacker0x01/react-datepicker#4481 - v7: https://github.com/Hacker0x01/react-datepicker/releases/tag/v7.0.0 BREAKING CHANGE: remove includeTime prop in favor of showTimeSelect BREAKING CHANGE: remove timeLabel prop in favor of timeCaption Refs: CDS-206

pullrequest bot reviewed Nov 26, 2023

View reviewed changes

pullrequest bot reviewed Nov 27, 2023

View reviewed changes

martijnrusschen reviewed Nov 27, 2023

View reviewed changes

src/stylesheets/datepicker.scss Show resolved Hide resolved

martijnrusschen reviewed Nov 27, 2023

View reviewed changes

src/stylesheets/datepicker.scss Show resolved Hide resolved

address review feedback

1456591

G07cha requested a review from martijnrusschen November 27, 2023 16:36

pullrequest bot reviewed Nov 27, 2023

View reviewed changes

remove unnecessary clonning of popperComponent

267c138

It was previously used to attach additonal props but that's no longer needed.

pullrequest bot reviewed Nov 28, 2023

View reviewed changes

martijnrusschen approved these changes Nov 28, 2023

View reviewed changes

pullrequest bot reviewed Dec 6, 2023

View reviewed changes

martijnrusschen merged commit 3cd50f6 into Hacker0x01:main Jan 3, 2024
6 checks passed

G07cha deleted the chore/migrate-to-floating-ui branch January 3, 2024 09:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

migrate from Popper.js to Floating-UI #4393

migrate from Popper.js to Floating-UI #4393

G07cha commented Nov 26, 2023

pullrequest bot left a comment

pullrequest bot left a comment •

edited

Loading

pullrequest bot left a comment

pullrequest bot left a comment

martijnrusschen Nov 27, 2023

G07cha Nov 27, 2023 •

edited

Loading

martijnrusschen Nov 27, 2023

G07cha Nov 27, 2023

pullrequest bot left a comment

pullrequest bot left a comment

codecov bot commented Nov 28, 2023

martijnrusschen left a comment

pullrequest bot left a comment

		@@ -160,11 +160,11 @@ export default class exampleComponents extends React.Component {
		<div>
		Full docs for the popper can be found at{" "}

migrate from Popper.js to Floating-UI #4393

migrate from Popper.js to Floating-UI #4393

Conversation

G07cha commented Nov 26, 2023

pullrequest bot left a comment

Choose a reason for hiding this comment

pullrequest bot left a comment • edited Loading

Choose a reason for hiding this comment

PullRequest Breakdown

pullrequest bot left a comment

Choose a reason for hiding this comment

pullrequest bot left a comment

Choose a reason for hiding this comment

martijnrusschen Nov 27, 2023

Choose a reason for hiding this comment

G07cha Nov 27, 2023 • edited Loading

Choose a reason for hiding this comment

martijnrusschen Nov 27, 2023

Choose a reason for hiding this comment

G07cha Nov 27, 2023

Choose a reason for hiding this comment

pullrequest bot left a comment

Choose a reason for hiding this comment

pullrequest bot left a comment

Choose a reason for hiding this comment

codecov bot commented Nov 28, 2023

Codecov Report

martijnrusschen left a comment

Choose a reason for hiding this comment

pullrequest bot left a comment

Choose a reason for hiding this comment

pullrequest bot left a comment •

edited

Loading

G07cha Nov 27, 2023 •

edited

Loading