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.

Sign up for GitHub

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

Jump to bottom

chore: Remove ally.js #5226

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

chore: Remove ally.js #5226

wants to merge 4 commits into from

Conversation

tlnd-mhuchet
Copy link
Contributor

What is the problem this PR is trying to solve?
We use ally.js to have a polyfill for :focus-within in css.

It is not really needed anymore as this pseudo class in now widely supported.

What is the chosen solution to this problem?
Remove ally.js polyfill and switch the usage of the lib css class to the pseudo class.

Please check if the PR fulfills these requirements

  • The PR have used yarn changeset to a request a release from the CI if wanted.
  • The PR commit message follows our guidelines
  • Tests for the changes have been added (for bug fixes / features) And non reg done before need review
  • Docs have been added / updated (for bug fixes / features)
  • Related design / discussions / pages (not in jira), if any, are all linked or available in the PR

[ ] This PR introduces a breaking change

@tlnd-mhuchet tlnd-mhuchet self-assigned this Mar 11, 2024
@github-actions GitHub Actions
Copy link
Contributor

5226

:octocat: Demo is available here

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Apr 15, 2024 •
edited

Title Lines Statements Branches Functions
assets-api Coverage: 28%
 28.4% (25/88) 30.76% (16/52) 21.42% (3/14)
cmf Coverage: 89%
 89.2% (1272/1426) 80.36% (618/769) 88.91% (361/406)
cmf-cqrs Coverage: 87%
 87.43% (160/183) 70.23% (59/84) 84.21% (48/57)
cmf-router Coverage: 70%
 69.23% (135/195) 55.71% (78/140) 56.81% (25/44)
components Coverage: 90%
 90.8% (5512/6070) 81.82% (3188/3896) 88.19% (1390/1576)
containers Coverage: 84%
 83.71% (1388/1658) 74.81% (692/925) 75.17% (327/435)
dataviz Coverage: 85%
 85.44% (323/378) 66.66% (160/240) 75.79% (119/157)
design-system Coverage: 67%
 66.88% (1002/1498) 50.38% (527/1046) 54.54% (216/396)
faceted-search Coverage: 85%
 85.69% (677/790) 79.31% (303/382) 82.63% (238/288)
flow-designer Coverage: 70%
 70.07% (651/929) 66.72% (355/532) 70.92% (200/282)
forms Coverage: 85%
 85.4% (1627/1905) 75.53% (920/1218) 84.22% (459/545)
http Coverage: 100%
 100% (85/85) 98.07% (51/52) 100% (34/34)
sagas Coverage: 92%
 92.3% (24/26) 66.66% (4/6) 50% (2/4)
stepper Coverage: 80%
 81.52% (150/184) 59.34% (54/91) 80.85% (38/47)
utils Coverage: 100%
 100% (73/73) 90.9% (10/11) 100% (24/24)

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Apr 15, 2024 •
edited

Size Change: -1.62 kB (0%)

Total Size: 17.7 MB

Filename Size Change
./packages/components/dist/TalendReactComponents.css 397 kB -130 B (0%)
./packages/components/dist/TalendReactComponents.js 4.84 MB -1.18 kB (0%)
./packages/components/dist/TalendReactComponents.js.dependencies.json 3.04 kB -170 B (-5%) ✅
./packages/components/dist/TalendReactComponents.min.css 211 kB -130 B (0%)
./packages/components/dist/TalendReactComponents.min.js.dependencies.json 3.12 kB -170 B (-5%) ✅
./packages/theme/dist/bootstrap.css 172 kB +209 B (0%)
ℹ️ View Unchanged
Filename Size Change
./packages/assets-api/dist/TalendAssetsApi.js 7.15 kB 0 B
./packages/assets-api/dist/TalendAssetsApi.js.dependencies.json 2 B 0 B
./packages/assets-api/dist/TalendAssetsApi.min.js 3.33 kB 0 B
./packages/assets-api/dist/TalendAssetsApi.min.js.dependencies.json 2 B 0 B
./packages/cmf-cqrs/dist/TalendReactCmfCqrs.js 277 kB 0 B
./packages/cmf-cqrs/dist/TalendReactCmfCqrs.js.dependencies.json 595 B 0 B
./packages/cmf-cqrs/dist/TalendReactCmfCqrs.min.js 54.6 kB 0 B
./packages/cmf-cqrs/dist/TalendReactCmfCqrs.min.js.dependencies.json 614 B 0 B
./packages/cmf-router/dist/TalendReactCmfRouter.js 163 kB 0 B
./packages/cmf-router/dist/TalendReactCmfRouter.js.dependencies.json 1.25 kB 0 B
./packages/cmf-router/dist/TalendReactCmfRouter.min.js 12.8 kB 0 B
./packages/cmf-router/dist/TalendReactCmfRouter.min.js.dependencies.json 1.29 kB 0 B
./packages/cmf/dist/TalendReactCmf.js 806 kB 0 B
./packages/cmf/dist/TalendReactCmf.js.dependencies.json 1.31 kB 0 B
./packages/cmf/dist/TalendReactCmf.min.js 151 kB 0 B
./packages/cmf/dist/TalendReactCmf.min.js.dependencies.json 1.35 kB 0 B
./packages/components/dist/TalendReactComponents.min.js 1.29 MB -48 B (0%)
./packages/containers/dist/TalendReactContainers.css 2.99 kB 0 B
./packages/containers/dist/TalendReactContainers.js 706 kB 0 B
./packages/containers/dist/TalendReactContainers.js.dependencies.json 1.4 kB 0 B
./packages/containers/dist/TalendReactContainers.min.css 1.78 kB 0 B
./packages/containers/dist/TalendReactContainers.min.js 136 kB 0 B
./packages/containers/dist/TalendReactContainers.min.js.dependencies.json 1.45 kB 0 B
./packages/dataviz/dist/TalendReactDataviz.css 28.1 kB 0 B
./packages/dataviz/dist/TalendReactDataviz.js 370 kB 0 B
./packages/dataviz/dist/TalendReactDataviz.js.dependencies.json 1.15 kB 0 B
./packages/dataviz/dist/TalendReactDataviz.min.css 12.2 kB 0 B
./packages/dataviz/dist/TalendReactDataviz.min.js 59.5 kB 0 B
./packages/dataviz/dist/TalendReactDataviz.min.js.dependencies.json 1.19 kB 0 B
./packages/design-system/dist/TalendDesignSystem.css 332 kB 0 B
./packages/design-system/dist/TalendDesignSystem.js 1.43 MB 0 B
./packages/design-system/dist/TalendDesignSystem.js.dependencies.json 1.49 kB 0 B
./packages/design-system/dist/TalendDesignSystem.min.css 214 kB 0 B
./packages/design-system/dist/TalendDesignSystem.min.js 288 kB 0 B
./packages/design-system/dist/TalendDesignSystem.min.js.dependencies.json 1.53 kB 0 B
./packages/design-tokens/dist/TalendDesignTokens.css 79.1 kB 0 B
./packages/design-tokens/dist/TalendDesignTokens.js 39.7 kB 0 B
./packages/design-tokens/dist/TalendDesignTokens.js.dependencies.json 2 B 0 B
./packages/design-tokens/dist/TalendDesignTokens.min.css 74.9 kB 0 B
./packages/design-tokens/dist/TalendDesignTokens.min.js 33.4 kB 0 B
./packages/design-tokens/dist/TalendDesignTokens.min.js.dependencies.json 2 B 0 B
./packages/faceted-search/dist/TalendReactFacetedSearch.css 19.1 kB 0 B
./packages/faceted-search/dist/TalendReactFacetedSearch.js 751 kB 0 B
./packages/faceted-search/dist/TalendReactFacetedSearch.js.dependencies.json 1.54 kB 0 B
./packages/faceted-search/dist/TalendReactFacetedSearch.min.css 6.28 kB 0 B
./packages/faceted-search/dist/TalendReactFacetedSearch.min.js 148 kB 0 B
./packages/faceted-search/dist/TalendReactFacetedSearch.min.js.dependencies.json 1.59 kB 0 B
./packages/flow-designer/dist/TalendReactFlowDesigner.min.js 50.7 kB 0 B
./packages/flow-designer/dist/TalendReactFlowDesigner.min.js.dependencies.json 1.25 kB 0 B
./packages/forms/dist/TalendReactForms.css 19.7 kB 0 B
./packages/forms/dist/TalendReactForms.js 1.06 MB 0 B
./packages/forms/dist/TalendReactForms.js.dependencies.json 1.36 kB 0 B
./packages/forms/dist/TalendReactForms.min.css 6.13 kB 0 B
./packages/forms/dist/TalendReactForms.min.js 324 kB 0 B
./packages/forms/dist/TalendReactForms.min.js.dependencies.json 1.39 kB 0 B
./packages/http/dist/TalendHttp.js 24.8 kB 0 B
./packages/http/dist/TalendHttp.js.dependencies.json 2 B 0 B
./packages/http/dist/TalendHttp.min.js 5.31 kB 0 B
./packages/http/dist/TalendHttp.min.js.dependencies.json 2 B 0 B
./packages/icons/dist/bundle.js 1.26 kB 0 B
./packages/icons/dist/info.js 26.8 kB 0 B
./packages/icons/dist/react.esm.js 614 kB 0 B
./packages/icons/dist/react.js 654 kB 0 B
./packages/icons/dist/talend-icons-webfont.css 18.3 kB 0 B
./packages/icons/dist/talendicons.css 334 B 0 B
./packages/icons/dist/TalendIcons.js 786 kB 0 B
./packages/icons/dist/TalendIcons.js.dependencies.json 128 B 0 B
./packages/icons/dist/TalendIcons.min.js 636 kB 0 B
./packages/icons/dist/TalendIcons.min.js.dependencies.json 131 B 0 B
./packages/icons/dist/typeUtils.js 14.5 kB 0 B
./packages/router-bridge/dist/TalendRouterBridge.js 134 kB 0 B
./packages/router-bridge/dist/TalendRouterBridge.js.dependencies.json 1.05 kB 0 B
./packages/router-bridge/dist/TalendRouterBridge.min.js 21 kB 0 B
./packages/router-bridge/dist/TalendRouterBridge.min.js.dependencies.json 1.07 kB 0 B
./packages/sagas/dist/TalendReactSagas.js 10.9 kB 0 B
./packages/sagas/dist/TalendReactSagas.js.dependencies.json 348 B 0 B
./packages/sagas/dist/TalendReactSagas.min.js 1.41 kB 0 B
./packages/sagas/dist/TalendReactSagas.min.js.dependencies.json 360 B 0 B
./packages/stepper/dist/TalendReactStepper.css 2.41 kB 0 B
./packages/stepper/dist/TalendReactStepper.js 97 kB 0 B
./packages/stepper/dist/TalendReactStepper.js.dependencies.json 1.27 kB 0 B
./packages/stepper/dist/TalendReactStepper.min.css 1.2 kB 0 B
./packages/stepper/dist/TalendReactStepper.min.js 9.37 kB 0 B
./packages/stepper/dist/TalendReactStepper.min.js.dependencies.json 1.31 kB 0 B
./packages/storybook-docs/dist/globalStyles.js 0 B 0 B 🆕
./packages/storybook-docs/dist/globalStyles.min.css 384 B 0 B
./packages/storybook-docs/dist/managerStyles.js 0 B 0 B 🆕
./packages/storybook-docs/dist/managerStyles.min.css 1.5 kB 0 B
./packages/theme/dist/bootstrap.js 2.51 kB 0 B
./packages/theme/dist/bootstrap.js.dependencies.json 3 B 0 B

compressed-size-action

jmfrancois
jmfrancois reviewed Apr 15, 2024
View reviewed changes
fork/module-to-cdn/modules.json
Comment on lines -224 to -232
"ally.js": {
"var": "ally",
"versions": {
">= 1.0.0": {
"development": "/ally.min.js",
"production": "/ally.min.js"
}
}
},
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would let it here in terms of backward compatiliby, in case you still have it in the code base it can break your app.
Remove it from the code and you should not have it anymore in the final application without this change.

jmfrancois
jmfrancois requested changes Apr 15, 2024
View reviewed changes
Copy link
Collaborator

@jmfrancois jmfrancois left a comment

Choose a reason for hiding this comment

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

Also you need to add coresponding changeset

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jmfrancois jmfrancois jmfrancois requested changes

@Gbacc Gbacc Gbacc approved these changes

Requested changes must be addressed to merge this pull request.

Assignees

@tlnd-mhuchet tlnd-mhuchet

Labels
need review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@tlnd-mhuchet @Gbacc @jmfrancois