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

feat(MyPortal): [#174801172] Screen component for region service external webview #2204

Merged
merged 10 commits into from
Sep 21, 2020

Conversation

CrisTofani
Copy link
Contributor

@CrisTofani CrisTofani commented Sep 17, 2020

Short description

This PR adds the screen component for rendering the external webview of a service

How to test

Set the MYPORTAL_ENABLED env variable to true and navigate to the screen from a custom CTA component from dev-server, with action value set to: ioit://SERVICE_WEBVIEW&?url=http://localhost:3000/myportal_playground.html

@pagopa-github-bot pagopa-github-bot changed the title [#174801172] Screen component for region service external webview feat: [#174801172] Screen component for region service external webview Sep 17, 2020
@pagopa-github-bot
Copy link
Collaborator

pagopa-github-bot commented Sep 17, 2020

Affected stories

  • 🌟 #174801172: Come CIT voglio navigare ad uno screen contenente la webview per la visualizzazione del form

Generated by 🚫 dangerJS against 00b0711

@codecov-commenter
Copy link

codecov-commenter commented Sep 17, 2020

Codecov Report

Merging #2204 into master will increase coverage by 0.01%.
The diff coverage is 53.44%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2204      +/-   ##
==========================================
+ Coverage   47.13%   47.15%   +0.01%     
==========================================
  Files         492      493       +1     
  Lines       14790    14845      +55     
  Branches     2774     3017     +243     
==========================================
+ Hits         6972     7000      +28     
- Misses       7776     7803      +27     
  Partials       42       42              
Impacted Files Coverage Δ
ts/navigation/routes.ts 100.00% <ø> (ø)
ts/screens/services/ServicesWebviewScreen.tsx 40.47% <40.47%> (ø)
ts/store/reducers/authentication.ts 49.23% <66.66%> (+0.84%) ⬆️
ts/store/reducers/internalRouteNavigation.ts 85.71% <66.66%> (-5.20%) ⬇️
ts/components/ui/Markdown/handlers/internalLink.ts 78.04% <100.00%> (+1.12%) ⬆️
ts/navigation/ServicesNavigator.ts 100.00% <100.00%> (ø)

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 fbba2eb...00b0711. Read the comment docs.

@pagopa-github-bot pagopa-github-bot changed the title feat: [#174801172] Screen component for region service external webview feat(MyPortal): [#174801172] Screen component for region service external webview Sep 18, 2020

const initialInternalRouteNavigationState = null;
const initialInternalRouteNavigationState = {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this change needed?
an empty object could be misleading if we want to represent that there is no internal route

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep you're right, i currently reverted the change


return {
url: maybeParams.fold("", p => p.url),
token: tokenFromSessionInfoSelector(state, "walletToken")
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
token: tokenFromSessionInfoSelector(state, "walletToken")
token: tokenFromNameSelector("walletToken")(state)

see other suggestion about selector

const [isCookieAvailable, setIsCookieAvailable] = React.useState(false);
const [cookieError, setCookieError] = React.useState(false);

const onDidFocus = () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

I did few test and this callback is never called (maybe a bug on navigation library)
Perhaps you could use React.useEffect with a clean-up callback. I tested it and it works

@@ -132,6 +136,17 @@ export const sessionInfoSelector = (state: GlobalState) =>
? some(state.authentication.sessionInfo)
: none;

export const tokenFromSessionInfoSelector = (
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
export const tokenFromSessionInfoSelector = (
export const tokenFromNameSelector = (
tokenName: keyof Omit<PublicSession, "spidLevel">
): ((state: GlobalState) => Option<string>) =>
createSelector<GlobalState, Option<PublicSession>, Option<string>>(
sessionInfoSelector,
maybeSessionInfo => maybeSessionInfo.map(si => si[tokenName])
);

@Undermaken Undermaken merged commit 4f861e4 into master Sep 21, 2020
@Undermaken Undermaken deleted the 174801172-region-service-webview-screen branch September 21, 2020 15:39
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.

4 participants