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

build: bump react-popper to version 2 #625

Merged
merged 1 commit into from
Jun 18, 2020
Merged

build: bump react-popper to version 2 #625

merged 1 commit into from
Jun 18, 2020

Conversation

xballoy
Copy link
Contributor

@xballoy xballoy commented Jun 5, 2020

Bump react-potter to latest version. I had to add @popperjs/core as dependency. See react-popper migration guide.

I refactored the unit tests of Popover to have functional tests so they do not depend on the implementation.

Changes :

  • data-placement becomes data-popper-placement so it might break CSS if people are overriding the default behavior
  • the arrow design in now in a ::before pseudo-element because Popper uses transform to position the arrow inside the popper

To override the design of the arrow you need to write the following CSS:

  .af-popover__container-pop {
    &[data-popper-placement^='top'] > .af-popover__arrow::before,
    &[data-popper-placement^='right'] > .af-popover__arrow::before,
    &[data-popper-placement^='bottom'] > .af-popover__arrow::before,
    &[data-popper-placement^='left'] > .af-popover__arrow::before {
      // Override background or write your custom CSS
      background: white;
      box-shadow: -2px 0 5px 0px rgba(0, 0, 0, 0.2);
    }
  }

@guillaume-chervet @samuel-gomez @youf-olivier

@xballoy xballoy marked this pull request as draft June 5, 2020 16:05
@xballoy xballoy marked this pull request as ready for review June 8, 2020 16:02
package-lock.json Outdated Show resolved Hide resolved
@xballoy xballoy marked this pull request as draft June 10, 2020 12:22
@xballoy xballoy marked this pull request as ready for review June 11, 2020 12:17
@@ -40,7 +40,7 @@
"@types/jest": "^23.3.11",
"@types/prop-types": "^15.5.8",
"@types/react": "^16.7.18",
"@types/react-modal": "^3.6.0",
"@types/react-modal": "3.8.0",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had to force the version because I don't know why but it always downloaded a newer version which breaks the modal package.

package.json Outdated
@@ -137,7 +137,7 @@
"react-router-dom": "4.1.0",
"react-scripts": "^1.1.5",
"react-syntax-highlighter": "^7.0.4",
"react-test-renderer": "^16.7.0",
"react-test-renderer": "^16.8.0",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The react-test-renderer version wasn't aligned with react and react-dom

position: absolute;
z-index: -1;

&::before {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The arrow is now in a ::before pseudo-element so I rewrote it so it's now easier to override

youf-olivier
youf-olivier previously approved these changes Jun 18, 2020
@youf-olivier
Copy link
Contributor

youf-olivier commented Jun 18, 2020

I Merged an older PR before this one, and so you have some conflicts

@xballoy
Copy link
Contributor Author

xballoy commented Jun 18, 2020

I updated the PR to solve the conflicts: rebase on master and remove the reference to AXA Nexus

youf-olivier
youf-olivier previously approved these changes Jun 18, 2020
@youf-olivier
Copy link
Contributor

Holly s... seriously ?

BREAKING CHANGE:

- The style of the arrow is now in a `::before` pseudo-element.
- `data-placement` of the Popover is now `data-popper-placement`.

See the migration guide for more information:
https://popper.js.org/docs/v2/migration-guide/
@youf-olivier youf-olivier merged commit 36958c3 into AxaFrance:master Jun 18, 2020
@xballoy xballoy deleted the feature/bumpReactPopper branch June 18, 2020 09:52
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.

None yet

4 participants