pagopa / io-app

IO, l'app dei servizi pubblici
https://io.italia.it
European Union Public License 1.2
568 stars 97 forks source link

chore(Cross): [IOAPPX-296] Remove `native-base` from dependencies #5558

Closed CrisTofani closed 3 weeks ago

CrisTofani commented 3 months ago

[!caution] This PR depends on #5506

Short description

This PR removes native-base and its related packages from dependencies.

List of changes proposed in this pull request

[!warning] Known issue Screens that use the BaseScreenComponent could be rendered with a huge amount of space on top when used in combination with SafeAreaView

For other issues, please refer to the discussion below

Credits

This PR is the result of:

How to test

Launch the app in the local environment and accurately test the most used flows by the citizens, especially:

pagopa-github-bot commented 3 months ago

Affected stories

Generated by :no_entry_sign: dangerJS against cb2bc47bcdd40033e2bcd144b09291a0f8de084c

dpulls[bot] commented 2 months ago

:tada: All dependencies have been resolved !

CrisTofani commented 1 month ago

e2e tests running here https://github.com/pagopa/io-app/actions/runs/8718945245

CrisTofani commented 1 month ago

@LazyAfternoons Apparently some library is still using the prop-type raising the error, btw react-native reduced this to a warning and not an error in the 0.71, so i guess we will anyway remove the patch in future

dmnplb commented 1 month ago

@LazyAfternoons @CrisTofani Also removed the react-native-iphone-x-helper dependency in the following commit: https://github.com/pagopa/io-app/pull/5558/commits/9642b1f01c6d246c7ddba9818a4aa1ce77e575ee

codecov[bot] commented 1 month ago

Codecov Report

Attention: Patch coverage is 31.42077% with 251 lines in your changes are missing coverage. Please review.

Project coverage is 49.59%. Comparing base (4f204b4) to head (cb2bc47). Report is 71 commits behind head on master.

Additional details and impacted files [![Impacted file tree graph](https://app.codecov.io/gh/pagopa/io-app/pull/5558/graphs/tree.svg?width=650&height=150&src=pr&token=zsurlZdPFW&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pagopa)](https://app.codecov.io/gh/pagopa/io-app/pull/5558?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pagopa) ```diff @@ Coverage Diff @@ ## master #5558 +/- ## ========================================== + Coverage 48.42% 49.59% +1.16% ========================================== Files 1488 1597 +109 Lines 31617 31805 +188 Branches 7669 7639 -30 ========================================== + Hits 15311 15774 +463 + Misses 16238 15977 -261 + Partials 68 54 -14 ``` | [Files](https://app.codecov.io/gh/pagopa/io-app/pull/5558?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pagopa) | Coverage Δ | | |---|---|---| | [ts/components/CalendarsListContainer.tsx](https://app.codecov.io/gh/pagopa/io-app/pull/5558?src=pr&el=tree&filepath=ts%2Fcomponents%2FCalendarsListContainer.tsx&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pagopa#diff-dHMvY29tcG9uZW50cy9DYWxlbmRhcnNMaXN0Q29udGFpbmVyLnRzeA==) | `9.25% <ø> (ø)` | | | [ts/components/ContextualInfo.tsx](https://app.codecov.io/gh/pagopa/io-app/pull/5558?src=pr&el=tree&filepath=ts%2Fcomponents%2FContextualInfo.tsx&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pagopa#diff-dHMvY29tcG9uZW50cy9Db250ZXh0dWFsSW5mby50c3g=) | `77.77% <ø> (-4.05%)` | :arrow_down: | | [ts/components/CopyButtonComponent.tsx](https://app.codecov.io/gh/pagopa/io-app/pull/5558?src=pr&el=tree&filepath=ts%2Fcomponents%2FCopyButtonComponent.tsx&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pagopa#diff-dHMvY29tcG9uZW50cy9Db3B5QnV0dG9uQ29tcG9uZW50LnRzeA==) | `75.00% <ø> (+4.41%)` | :arrow_up: | | [ts/components/FiscalCodeLandscapeOverlay.tsx](https://app.codecov.io/gh/pagopa/io-app/pull/5558?src=pr&el=tree&filepath=ts%2Fcomponents%2FFiscalCodeLandscapeOverlay.tsx&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pagopa#diff-dHMvY29tcG9uZW50cy9GaXNjYWxDb2RlTGFuZHNjYXBlT3ZlcmxheS50c3g=) | `9.52% <ø> (ø)` | | | [ts/components/IdpsGrid.tsx](https://app.codecov.io/gh/pagopa/io-app/pull/5558?src=pr&el=tree&filepath=ts%2Fcomponents%2FIdpsGrid.tsx&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pagopa#diff-dHMvY29tcG9uZW50cy9JZHBzR3JpZC50c3g=) | `38.46% <100.00%> (ø)` | | | [...omponents/LabelledItem/LabelledItemIconOrImage.tsx](https://app.codecov.io/gh/pagopa/io-app/pull/5558?src=pr&el=tree&filepath=ts%2Fcomponents%2FLabelledItem%2FLabelledItemIconOrImage.tsx&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pagopa#diff-dHMvY29tcG9uZW50cy9MYWJlbGxlZEl0ZW0vTGFiZWxsZWRJdGVtSWNvbk9ySW1hZ2UudHN4) | `100.00% <100.00%> (ø)` | | | [ts/components/LabelledItem/index.tsx](https://app.codecov.io/gh/pagopa/io-app/pull/5558?src=pr&el=tree&filepath=ts%2Fcomponents%2FLabelledItem%2Findex.tsx&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pagopa#diff-dHMvY29tcG9uZW50cy9MYWJlbGxlZEl0ZW0vaW5kZXgudHN4) | `84.61% <100.00%> (-3.85%)` | :arrow_down: | | [ts/components/LoadingErrorComponent.tsx](https://app.codecov.io/gh/pagopa/io-app/pull/5558?src=pr&el=tree&filepath=ts%2Fcomponents%2FLoadingErrorComponent.tsx&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pagopa#diff-dHMvY29tcG9uZW50cy9Mb2FkaW5nRXJyb3JDb21wb25lbnQudHN4) | `100.00% <100.00%> (ø)` | | | [ts/components/LoadingSpinnerOverlay.tsx](https://app.codecov.io/gh/pagopa/io-app/pull/5558?src=pr&el=tree&filepath=ts%2Fcomponents%2FLoadingSpinnerOverlay.tsx&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pagopa#diff-dHMvY29tcG9uZW50cy9Mb2FkaW5nU3Bpbm5lck92ZXJsYXkudHN4) | `100.00% <ø> (ø)` | | | [ts/components/OrganizationHeader.tsx](https://app.codecov.io/gh/pagopa/io-app/pull/5558?src=pr&el=tree&filepath=ts%2Fcomponents%2FOrganizationHeader.tsx&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pagopa#diff-dHMvY29tcG9uZW50cy9Pcmdhbml6YXRpb25IZWFkZXIudHN4) | `100.00% <100.00%> (ø)` | | | ... and [200 more](https://app.codecov.io/gh/pagopa/io-app/pull/5558?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pagopa) | | ... and [288 files with indirect coverage changes](https://app.codecov.io/gh/pagopa/io-app/pull/5558/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pagopa) ------ [Continue to review full report in Codecov by Sentry](https://app.codecov.io/gh/pagopa/io-app/pull/5558?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pagopa). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pagopa) > `Δ = absolute (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://app.codecov.io/gh/pagopa/io-app/pull/5558?dropdown=coverage&src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pagopa). Last update [9287ace...cb2bc47](https://app.codecov.io/gh/pagopa/io-app/pull/5558?dropdown=coverage&src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pagopa). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pagopa).
dmnplb commented 1 month ago

Tested on the main flows I have only encountered a little issue on IDPay IDPAY_ONBOARDING_INITIATIVE_DETAILS screen not necessarily related to the native-based.

@hevelius Good catch! It's probably caused by the edits made in this PR. Let me check 👀

dmnplb commented 1 month ago

@hevelius Changed IDPay related screens in the following commit → https://github.com/pagopa/io-app/pull/5558/commits/a6077741cf2eb5ff9f7ba9a301d2d7b5e201c352 Changes applied:

I have tested the entire onboarding flow and it seems to work well (cc @mastro993)

CrisTofani commented 1 month ago

I'm not able to login with Test accounts anymore both on Android and iOS? It seems that text inputs are disabled in some way. Can we do a double check?

Demo testlogin-ko.mov

@shadowsheep1 what do you think if we change the components input with the new TextInput component?

shadowsheep1 commented 1 month ago

I'm not able to login with Test accounts anymore both on Android and iOS? It seems that text inputs are disabled in some way. Can we do a double check? Demo testlogin-ko.mov

@shadowsheep1 what do you think if we change the components input with the new TextInput component?

No problem! We could even use the one from DS ;)

CrisTofani commented 1 month ago

I'm not able to login with Test accounts anymore both on Android and iOS? It seems that text inputs are disabled in some way. Can we do a double check? Demo testlogin-ko.mov

@shadowsheep1 what do you think if we change the components input with the new TextInput component?

No problem! We could even use the one from DS ;)

@shadowsheep1 addressed in 101996f

shadowsheep1 commented 1 month ago

I'm not able to login with Test accounts anymore both on Android and iOS? It seems that text inputs are disabled in some way. Can we do a double check? Demo testlogin-ko.mov

@shadowsheep1 what do you think if we change the components input with the new TextInput component?

No problem! We could even use the one from DS ;)

@shadowsheep1 addressed in 101996f

Superb! ❤️

dmnplb commented 3 weeks ago

We should check the Payments and Messages related flows to be safe. If nothing is broken, we should merge. What do you think?

shadowsheep1 commented 3 weeks ago

We should check the Payments and Messages related flows to be safe. If nothing is broken, we should merge. What do you think?

It seems that the testID on TextInputValidation and TextInputPassword is not well propagated... what do you think?

<TextInput
  accessibilityLabel="Password"
  accessibilityState={
    Object {
      "disabled": false,
    }
  }
  accessible={true}
  blurOnSubmit={true}
  cursorColor="#0B3EE3"
  disableFullscreenUI={true}
  editable={true}
  inputMode="password"
  onBlur={[Function]}
  onChangeText={[Function]}
  onFocus={[Function]}
  returnKeyType="done"
  secureTextEntry={true}
  selectionColor="#0B3EE3"
  style={
    Array [
      Object {
        "flex": 1,
        "fontSize": 16,
        "height": "100%",
        "lineHeight": 24,
        "marginTop": 8,
      },
      Object {
        "fontFamily": "Titillium Web",
        "fontStyle": "normal",
        "fontWeight": "600",
      },
    ]
  }
  value=""
/>
dmnplb commented 3 weeks ago

It seems that the testID on TextInputValidation and TextInputPassword is not well propagated... what do you think?

@shadowsheep1 A fix has been already released in the io-app-design-system package:

The tests will be fixed soon, but on the authentication side we should be safe thanks to your review

CrisTofani commented 3 weeks ago

We should check the Payments and Messages related flows to be safe. If nothing is broken, we should merge. What do you think?

It seems that the testID on TextInputValidation and TextInputPassword is not well propagated... what do you think?

@shadowsheep1 Addressed in the last 2 commits now it should be fine