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

[#172526356] Adds Screen to explain why CIE login is not available #1769

Merged
merged 23 commits into from
Jun 25, 2020
Merged

[#172526356] Adds Screen to explain why CIE login is not available #1769

merged 23 commits into from
Jun 25, 2020

Conversation

CrisTofani
Copy link
Contributor

Short description:
This PR adds the screen that explains why CIE is not supported

List of changes proposed in this pull request:

  • ts/store/actions/cie.ts
  • ts/store/reducers/cie.ts
  • ts/sagas/cie.ts
  • ts/screens/authentication/LandingScreen.tsx

Android

iOS

@pagopa-github-bot
Copy link
Collaborator

pagopa-github-bot commented May 6, 2020 •
edited
Loading

Affected stories

  • 🌟 #172526356: Come CIT vorrei conoscere i motivi per cui non posso entrare con CIE

Generated by 🚫 dangerJS

@CrisTofani CrisTofani marked this pull request as draft May 6, 2020 10:04
@CrisTofani CrisTofani marked this pull request as ready for review May 6, 2020 10:09
Undermaken
Undermaken suggested changes May 12, 2020
View reviewed changes
locales/it/cie/cieNotSupported.md Outdated
@@ -0,0 +1 @@
Per effettuare il login è necessario avere un dispositivo con tecnologia NFC o un dispositivo compatibile con l’attuale implementazione (i dispositivi iOS non sono ancora compatibili ma a breve lo diventeranno con il nuovo aggiornamento).
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Per effettuare il login è necessario avere un dispositivo con tecnologia NFC o un dispositivo compatibile con l’attuale implementazione (i dispositivi iOS non sono ancora compatibili ma a breve lo diventeranno con il nuovo aggiornamento).
Per effettuare il login è necessario avere un dispositivo con tecnologia NFC o un dispositivo compatibile con l’attuale implementazione (i dispositivi iOS non sono ancora compatibili ma a breve lo diventeranno con i prossimi aggiornamenti).

I guess it's more appropriate. In that way the user could expect CIE in iOS on the next release

ts/screens/authentication/LandingScreen.tsx Outdated
@@ -120,15 +136,86 @@ class LandingScreen extends React.PureComponent<Props> {
}
}

private renderAndroidConditions = () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Could it be a component included in its own file?
We should pass to it hasCieApiLevelSupport

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We may create a component for the complete child of the Modal including this block maybe?

          <React.Fragment>
            <Markdown>
              {I18n.t("authentication.landing.cie_unsupported.body")}
            </Markdown>
            {Platform.OS === "android" && this.renderAndroidConditions()}
          </React.Fragment>

we should pass hasCieApiLevelSupport and hasCieNFCFeature too i guess

Copy link
Contributor Author

@CrisTofani CrisTofani May 13, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

@Undermaken
The component is available in #05798b8

Undermaken
Undermaken suggested changes May 13, 2020
View reviewed changes
locales/it/cie/cieNotSupported.md Outdated
@@ -0,0 +1 @@
Per effettuare il login è necessario avere un dispositivo con tecnologia NFC o un dispositivo compatibile con l’attuale implementazione (i dispositivi iOS non sono ancora compatibili ma a breve lo diventeranno con i prossimi aggiornamenti).
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Per effettuare il login è necessario avere un dispositivo con tecnologia NFC o un dispositivo compatibile con l’attuale implementazione (i dispositivi iOS non sono ancora compatibili ma a breve lo diventeranno con i prossimi aggiornamenti).
Per effettuare il login è necessario avere un dispositivo con tecnologia NFC e il sistema operativo con i requisiti minimi necessari (i dispositivi **iOS** non sono ancora compatibili ma a breve lo diventeranno con i prossimi aggiornamenti).

This description, in the way we proposed, doesn't make sense

ts/components/cie/CieNotSupported.tsx Outdated
<ListItem>
<IconFont
name="io-tick-big"
size={16}
Copy link
Contributor

Choose a reason for hiding this comment

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

Avoid magic number
Place it as const on top of file

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 6533ccb

ts/screens/authentication/LandingScreen.tsx Outdated
noCie: {
opacity: 0.35
},
checkboxBackground: {
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems to be not used

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 6533ccb

ts/store/reducers/cie.ts Outdated
case getType(hasApiLevelSupport.failure):
return {
...state,
hasApiLevelSupport: pot.toError(state.isCieSupported, action.payload)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
hasApiLevelSupport: pot.toError(state.isCieSupported, action.payload)
hasApiLevelSupport: pot.toError(state.hasApiLevelSupport, action.payload)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 6533ccb

ts/store/reducers/cie.ts Outdated
case getType(hasNFCFeature.failure):
return {
...state,
hasNFCFeature: pot.toError(state.isCieSupported, action.payload)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
hasNFCFeature: pot.toError(state.isCieSupported, action.payload)
hasNFCFeature: pot.toError(state.hasNFCFeature, action.payload)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 6533ccb

@CrisTofani
Copy link
Contributor Author

Aggiungo le gif che mostrano la visualizzazione in condizione del supporto alla CIE

@Undermaken Undermaken requested a review from fabriziofff June 25, 2020 13:39
@Undermaken Undermaken merged commit 5bc9437 into master Jun 25, 2020
@CrisTofani CrisTofani deleted the 172526356-no-cie-support-screen branch March 23, 2021 11:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fabriziofff fabriziofff fabriziofff approved these changes

@Undermaken Undermaken Undermaken approved these changes

@francescopersico francescopersico Awaiting requested review from francescopersico

@matteodesanti matteodesanti Awaiting requested review from matteodesanti

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@CrisTofani @pagopa-github-bot @Undermaken @fabriziofff