Skip to content
This repository was archived by the owner on Mar 4, 2020. It is now read-only.

chore: move FocusZone components to react-bindings #2004

Merged
merged 4 commits into from
Oct 4, 2019

Conversation

layershifter
Copy link
Member

@layershifter layershifter commented Oct 3, 2019

↩️ Move of FocusZone

This PR moves FocusZone, AutoFocusZone, FocusTrapZone, related types and utils to @stardust-ui/react-bindings to make them reusable.

The main motivation for this is too avoid circular dependency: @stardust-ui/react-bindings => @stardust-ui/react => @stardust-ui/react-bindings

@codecov
Copy link

codecov bot commented Oct 4, 2019

Codecov Report

Merging #2004 into master will decrease coverage by 0.38%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2004      +/-   ##
==========================================
- Coverage   77.09%   76.71%   -0.39%     
==========================================
  Files         160      160              
  Lines        5493     5492       -1     
  Branches     1603     1585      -18     
==========================================
- Hits         4235     4213      -22     
- Misses       1245     1266      +21     
  Partials       13       13
Impacted Files Coverage Δ
...ages/react/src/components/MenuButton/focusUtils.ts 20% <ø> (ø) ⬆️
...ges/react-bindings/src/FocusZone/AutoFocusZone.tsx 100% <ø> (ø)
...ges/react-bindings/src/FocusZone/focusUtilities.ts 87.67% <ø> (ø)
...ckages/react/src/components/Popup/PopupContent.tsx 95.45% <ø> (ø) ⬆️
packages/react/src/lib/mergeThemes.ts 100% <ø> (ø) ⬆️
...ackages/react/src/lib/felaInvokeKeyframesPlugin.ts 100% <ø> (ø) ⬆️
...ges/react-bindings/src/FocusZone/FocusTrapZone.tsx 65.15% <ø> (ø)
packages/react/src/components/Menu/MenuItem.tsx 48.48% <ø> (ø) ⬆️
packages/react/src/components/Icon/Icon.tsx 100% <ø> (ø) ⬆️
packages/react/src/components/Popup/Popup.tsx 67.4% <ø> (ø) ⬆️
... and 12 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2415040...e29883f. Read the comment docs.

@DustyTheBot
Copy link
Collaborator

Warnings
⚠️ There are no updates provided to CHANGELOG. Ensure there are no publicly visible changes introduced by this PR.
⚠️ Package (or peer) dependencies changed. Make sure you have approval before merging!

Changed dependencies are detected.

Changed dependencies in packages/react-bindings/package.json

package before after
@stardust-ui/accessibility - ^0.39.0
@stardust-ui/state - ^0.39.0

Generated by 🚫 dangerJS

"@babel/runtime": "^7.1.2"
"@babel/runtime": "^7.1.2",
"@stardust-ui/accessibility": "^0.39.0",
"@stardust-ui/state": "^0.39.0"
Copy link
Member Author

Choose a reason for hiding this comment

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

These packages can't be in optional dependencies because TS will throw on typings...

export const getDefinedProps = <Props extends Record<string, any>>(
props: Props,
): Partial<Props> => {
const getDefinedProps = <Props extends Record<string, any>>(props: Props): Partial<Props> => {
Copy link
Member Author

Choose a reason for hiding this comment

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

Should not be exported, nit

@@ -1,4 +1,3 @@
// https://jsperf.com/startdust-callable
Copy link
Member Author

Choose a reason for hiding this comment

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

Link is dead, 404

@@ -1,10 +1,10 @@
import { callable } from '@stardust-ui/react-bindings'
Copy link
Member Author

Choose a reason for hiding this comment

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

I also have to move callable as it used in FZ

@layershifter layershifter merged commit db0932f into master Oct 4, 2019
@layershifter layershifter deleted the chore/move-focuszone branch October 4, 2019 13:00
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants