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(IT Wallet): [SIW-667] Add details screen for an issued credential #5219

Merged
merged 4 commits into from
Nov 12, 2023

Conversation

hevelius
Copy link
Contributor

Short description

This PR adds a detail screen to show an issued credential info.

List of changes proposed in this pull request

  • Add ItwCredentialDetailsScreen

How to test

Activate the wallet then try to add a new credential from the wallet home screen. After issuing from the wallet home screen try a single tap on the credential card image to show related details. Of course the longpress on the credential card image should work to try a presentation flow.

@pagopa-github-bot pagopa-github-bot changed the title [SIW-667] Add details screen for an issued credential feat(IT Wallet): [SIW-667] Add details screen for an issued credential Nov 11, 2023
@pagopa-github-bot
Copy link
Collaborator

pagopa-github-bot commented Nov 11, 2023

Affected stories

  • 🌟 SIW-667: Come UTENTE voglio visualizzare il dettaglio della credenziale così da leggerne i vari campi

Generated by 🚫 dangerJS against 10660f8

Copy link

codecov bot commented Nov 11, 2023

Codecov Report

Merging #5219 (89605ba) into bundle/it-wallet (39388f3) will decrease coverage by 0.02%.
The diff coverage is 10.00%.

❗ Current head 89605ba differs from pull request most recent head 10660f8. Consider uploading reports for the commit 10660f8 to get more accurate results

Additional details and impacted files

Impacted file tree graph

@@                 Coverage Diff                  @@
##           bundle/it-wallet    #5219      +/-   ##
====================================================
- Coverage             45.69%   45.68%   -0.02%     
====================================================
  Files                  1618     1619       +1     
  Lines                 33395    33405      +10     
  Branches               8246     8246              
====================================================
+ Hits                  15261    15262       +1     
- Misses                18085    18094       +9     
  Partials                 49       49              
Files Coverage Δ
ts/features/it-wallet/navigation/ItwRoutes.ts 100.00% <ø> (ø)
...eatures/it-wallet/navigation/ItwStackNavigator.tsx 66.66% <ø> (ø)
.../credential/issuing/ItwCredentialPreviewScreen.tsx 2.43% <ø> (ø)
ts/features/it-wallet/screens/ItwHomeScreen.tsx 8.33% <0.00%> (-0.24%) ⬇️
.../screens/credential/ItwCredentialDetailsScreen.tsx 11.11% <11.11%> (ø)

Continue to review full report in Codecov by Sentry.

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

@hevelius hevelius marked this pull request as ready for review November 11, 2023 19:18
@hevelius hevelius requested a review from a team as a code owner November 11, 2023 19:18
accessibilityLabel: I18n.t(
"features.itWallet.presentation.credentialDetails.buttons.qrCode"
),
onPress: () => null
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we add a navigation to the missing feature screen?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is already a task on jira related to the missing screen. I would then like to manage everything in a single PR / activity.

Copy link
Contributor

@LazyAfternoons LazyAfternoons Nov 12, 2023

Choose a reason for hiding this comment

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

All right, the activity is to add it where it's missing but I thought we could start using it already on new screens since the screen is already ready and added to the navigator. Up to you anyway, no big deal.

action={I18n.t(
"features.itWallet.issuing.credentialPreviewScreen.somethingWrong.action"
)}
onPress={constNull}
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As above

Comment on lines 75 to 79
content={I18n.t(
"features.itWallet.issuing.credentialPreviewScreen.somethingWrong.subtitle",
{
issuer: credential.parsedCredential
}
Copy link
Contributor

@LazyAfternoons LazyAfternoons Nov 12, 2023

Choose a reason for hiding this comment

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

I think the content here should be slightly different. Also it currently does not display the issuer data since we are not storing it with the credential but it's not really needed according to this FIGMA.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure I understand. Anyway the figma you linked refers to a new version of the credential detail that we have not yet explored and which changes completely compared to the current version. I'm also not sure if there are all the components needed to make it. Related to the issuerName of the bottom banner I suggest to change locale string removing params because as figma in the support banner there are no mention about the issuer name.

Copy link
Contributor

@LazyAfternoons LazyAfternoons Nov 12, 2023

Choose a reason for hiding this comment

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

The current version is using the locale from the preview screen which has a different banner (which includes the issuer name) from the FIGMA I mentioned. Let's leave out the redesign of the whole screen, we can still just implement the banner with the new copy (which removes the issuer name and has a different copy) and keep the current design. The very old version doesn't have the banner at all so we either remove it completely or we can add it and use the new copy version. What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

here 10660f8

@@ -20,6 +20,7 @@ export const ITW_ROUTES = {
} as const,
PRESENTATION: {
PID_DETAILS: "ITW_PRESENTATION_PID_DETAILS",
CREDENTIAL_DETAILS: "ITW_CREDENTIAL_DETAILS",
Copy link
Contributor

@LazyAfternoons LazyAfternoons Nov 12, 2023

Choose a reason for hiding this comment

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

Given that we want to separate PID from CREDENTIALS, what do you think about adding this screen under CREDENTIALS -> PRESENTATION -> CREDENTIAL_DETAILS?

The PID section will be refactored in the future but we can start structuring the CREDENTIALS part differently.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

here 6d70ba3

Copy link
Contributor

@LazyAfternoons LazyAfternoons left a comment

Choose a reason for hiding this comment

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

LGTM

@hevelius hevelius merged commit 1ba2e67 into bundle/it-wallet Nov 12, 2023
4 checks passed
@hevelius hevelius deleted the SIW-667-credential-details-screen branch November 12, 2023 13:46
CrisTofani added a commit that referenced this pull request Jan 19, 2024
…5339)

Bumps [aiohttp](https://github.com/aio-libs/aiohttp) from 3.8.6 to
3.9.0.
<details>
<summary>Release notes</summary>
<p><em>Sourced from <a
href="https://github.com/aio-libs/aiohttp/releases">aiohttp's
releases</a>.</em></p>
<blockquote>
<h2>3.9.0</h2>
<h2>Features</h2>
<ul>
<li>
<p>Introduced <code>AppKey</code> for static typing support of
<code>Application</code> storage.
See <a
href="https://docs.aiohttp.org/en/stable/web_advanced.html#application-s-config">https://docs.aiohttp.org/en/stable/web_advanced.html#application-s-config</a></p>
<p>(<a
href="https://github.com/aio-libs/aiohttp/issues/5864">#5864</a>)</p>
</li>
<li>
<p>Added a graceful shutdown period which allows pending tasks to
complete before the application's cleanup is called.
The period can be adjusted with the <code>shutdown_timeout</code>
parameter. -- by :user:<code>Dreamsorcerer</code>.
See <a
href="https://docs.aiohttp.org/en/latest/web_advanced.html#graceful-shutdown">https://docs.aiohttp.org/en/latest/web_advanced.html#graceful-shutdown</a></p>
<p>(<a
href="https://github.com/aio-libs/aiohttp/issues/7188">#7188</a>)</p>
</li>
<li>
<p>Added <code>handler_cancellation
&lt;https://docs.aiohttp.org/en/stable/web_advanced.html#web-handler-cancellation&gt;</code>_
parameter to cancel web handler on client disconnection. -- by
:user:<code>mosquito</code>
This (optionally) reintroduces a feature removed in a previous release.
Recommended for those looking for an extra level of protection against
denial-of-service attacks.</p>
<p>(<a
href="https://github.com/aio-libs/aiohttp/issues/7056">#7056</a>)</p>
</li>
<li>
<p>Added support for setting response header parameters
<code>max_line_size</code> and <code>max_field_size</code>.</p>
<p>(<a
href="https://github.com/aio-libs/aiohttp/issues/2304">#2304</a>)</p>
</li>
<li>
<p>Added <code>auto_decompress</code> parameter to
<code>ClientSession.request</code> to override
<code>ClientSession._auto_decompress</code>. -- by
:user:<code>Daste745</code></p>
<p>(<a
href="https://github.com/aio-libs/aiohttp/issues/3751">#3751</a>)</p>
</li>
<li>
<p>Changed <code>raise_for_status</code> to allow a coroutine.</p>
<p>(<a
href="https://github.com/aio-libs/aiohttp/issues/3892">#3892</a>)</p>
</li>
<li>
<p>Added client brotli compression support (optional with runtime
check).</p>
<p>(<a
href="https://github.com/aio-libs/aiohttp/issues/5219">#5219</a>)</p>
</li>
<li>
<p>Added <code>client_max_size</code> to
<code>BaseRequest.clone()</code> to allow overriding the request body
size. -- :user:<code>anesabml</code>.</p>
<p>(<a
href="https://github.com/aio-libs/aiohttp/issues/5704">#5704</a>)</p>
</li>
<li>
<p>Added a middleware type alias
<code>aiohttp.typedefs.Middleware</code>.</p>
<p>(<a
href="https://github.com/aio-libs/aiohttp/issues/5898">#5898</a>)</p>
</li>
<li>
<p>Exported <code>HTTPMove</code> which can be used to catch any
redirection request
that has a location -- :user:<code>dreamsorcerer</code>.</p>
<p>(<a
href="https://github.com/aio-libs/aiohttp/issues/6594">#6594</a>)</p>
</li>
</ul>
<!-- raw HTML omitted -->
</blockquote>
<p>... (truncated)</p>
</details>
<details>
<summary>Changelog</summary>
<p><em>Sourced from <a
href="https://github.com/aio-libs/aiohttp/blob/master/CHANGES.rst">aiohttp's
changelog</a>.</em></p>
<blockquote>
<h1>3.9.0 (2023-11-18)</h1>
<h2>Features</h2>
<ul>
<li>
<p>Introduced <code>AppKey</code> for static typing support of
<code>Application</code> storage.
See <a
href="https://docs.aiohttp.org/en/stable/web_advanced.html#application-s-config">https://docs.aiohttp.org/en/stable/web_advanced.html#application-s-config</a></p>
<p><code>[#5864](aio-libs/aiohttp#5864)
&lt;https://github.com/aio-libs/aiohttp/issues/5864&gt;</code>_</p>
</li>
<li>
<p>Added a graceful shutdown period which allows pending tasks to
complete before the application's cleanup is called.
The period can be adjusted with the <code>shutdown_timeout</code>
parameter. -- by :user:<code>Dreamsorcerer</code>.
See <a
href="https://docs.aiohttp.org/en/latest/web_advanced.html#graceful-shutdown">https://docs.aiohttp.org/en/latest/web_advanced.html#graceful-shutdown</a></p>
<p><code>[#7188](aio-libs/aiohttp#7188)
&lt;https://github.com/aio-libs/aiohttp/issues/7188&gt;</code>_</p>
</li>
<li>
<p>Added <code>handler_cancellation
&lt;https://docs.aiohttp.org/en/stable/web_advanced.html#web-handler-cancellation&gt;</code>_
parameter to cancel web handler on client disconnection. -- by
:user:<code>mosquito</code>
This (optionally) reintroduces a feature removed in a previous release.
Recommended for those looking for an extra level of protection against
denial-of-service attacks.</p>
<p><code>[#7056](aio-libs/aiohttp#7056)
&lt;https://github.com/aio-libs/aiohttp/issues/7056&gt;</code>_</p>
</li>
<li>
<p>Added support for setting response header parameters
<code>max_line_size</code> and <code>max_field_size</code>.</p>
<p><code>[#2304](aio-libs/aiohttp#2304)
&lt;https://github.com/aio-libs/aiohttp/issues/2304&gt;</code>_</p>
</li>
<li>
<p>Added <code>auto_decompress</code> parameter to
<code>ClientSession.request</code> to override
<code>ClientSession._auto_decompress</code>. -- by
:user:<code>Daste745</code></p>
<p><code>[#3751](aio-libs/aiohttp#3751)
&lt;https://github.com/aio-libs/aiohttp/issues/3751&gt;</code>_</p>
</li>
<li>
<p>Changed <code>raise_for_status</code> to allow a coroutine.</p>
<p><code>[#3892](aio-libs/aiohttp#3892)
&lt;https://github.com/aio-libs/aiohttp/issues/3892&gt;</code>_</p>
</li>
<li>
<p>Added client brotli compression support (optional with runtime
check).</p>
<p><code>[#5219](aio-libs/aiohttp#5219)
&lt;https://github.com/aio-libs/aiohttp/issues/5219&gt;</code>_</p>
</li>
<li>
<p>Added <code>client_max_size</code> to
<code>BaseRequest.clone()</code> to allow overriding the request body
size. -- :user:<code>anesabml</code>.</p>
<p><code>[#5704](aio-libs/aiohttp#5704)
&lt;https://github.com/aio-libs/aiohttp/issues/5704&gt;</code>_</p>
</li>
<li>
<p>Added a middleware type alias
<code>aiohttp.typedefs.Middleware</code>.</p>
<p><code>[#5898](aio-libs/aiohttp#5898)
&lt;https://github.com/aio-libs/aiohttp/issues/5898&gt;</code>_</p>
</li>
<li>
<p>Exported <code>HTTPMove</code> which can be used to catch any
redirection request
that has a location -- :user:<code>dreamsorcerer</code>.</p>
</li>
</ul>
<!-- raw HTML omitted -->
</blockquote>
<p>... (truncated)</p>
</details>
<details>
<summary>Commits</summary>
<ul>
<li><a
href="https://github.com/aio-libs/aiohttp/commit/45b2c2c5773f0ee0d35fce8ff5716c78e91d9135"><code>45b2c2c</code></a>
Release v3.9.0 (<a
href="https://github.com/aio-libs/aiohttp/issues/7843">#7843</a>)</li>
<li><a
href="https://github.com/aio-libs/aiohttp/commit/5d59d3d6ac073a7db5e5d2234e03a67da5dec48a"><code>5d59d3d</code></a>
Release v3.9.0rc0 (<a
href="https://github.com/aio-libs/aiohttp/issues/7840">#7840</a>)</li>
<li><a
href="https://github.com/aio-libs/aiohttp/commit/c806814a8aaad1661d75e6e2b8d619d6c44d331d"><code>c806814</code></a>
Release v3.9.0rc0 (<a
href="https://github.com/aio-libs/aiohttp/issues/7838">#7838</a>)</li>
<li><a
href="https://github.com/aio-libs/aiohttp/commit/e07a1bdaacfb83fda3ea8f668edacb36c6c125df"><code>e07a1bd</code></a>
Use timestamp instead of datetime to achieve faster cookie expiration…
(<a
href="https://github.com/aio-libs/aiohttp/issues/7837">#7837</a>)</li>
<li><a
href="https://github.com/aio-libs/aiohttp/commit/53476dfd4ef4fb1bb74a267714bbc39eda71b403"><code>53476df</code></a>
Disallow arbitrary sequence types in version (<a
href="https://github.com/aio-libs/aiohttp/issues/7835">#7835</a>)
(<a
href="https://github.com/aio-libs/aiohttp/issues/7836">#7836</a>)</li>
<li><a
href="https://github.com/aio-libs/aiohttp/commit/9d712f2f9c06f71d48e98374938813643503bc34"><code>9d712f2</code></a>
Bump mypy from 1.6.1 to 1.7.0 (<a
href="https://github.com/aio-libs/aiohttp/issues/7833">#7833</a>)</li>
<li><a
href="https://github.com/aio-libs/aiohttp/commit/63a805e1d8360fd388b6e6443cd9bdfb139e90ea"><code>63a805e</code></a>
Bump python-on-whales from 0.66.0 to 0.67.0 (<a
href="https://github.com/aio-libs/aiohttp/issues/7832">#7832</a>)</li>
<li><a
href="https://github.com/aio-libs/aiohttp/commit/cb94533dd50426809b7fcbb8bbad0ef17509de5c"><code>cb94533</code></a>
Ensure writer is always reset on completion (<a
href="https://github.com/aio-libs/aiohttp/issues/7815">#7815</a>)
(<a
href="https://github.com/aio-libs/aiohttp/issues/7826">#7826</a>)</li>
<li><a
href="https://github.com/aio-libs/aiohttp/commit/c0f9017a9a34a7823e1ea9b9abb393bd6c10777b"><code>c0f9017</code></a>
[PR <a
href="https://github.com/aio-libs/aiohttp/issues/7821">#7821</a>/366ba40f
backport][3.9] Only check origin if insecure scheme and th...</li>
<li><a
href="https://github.com/aio-libs/aiohttp/commit/9d498ca1e632fe1976ea1dae0ea083b29b0cc4c0"><code>9d498ca</code></a>
Bump sphinx from 7.1.1 to 7.2.6 (<a
href="https://github.com/aio-libs/aiohttp/issues/7606">#7606</a>)</li>
<li>Additional commits viewable in <a
href="https://github.com/aio-libs/aiohttp/compare/v3.8.6...v3.9.0">compare
view</a></li>
</ul>
</details>
<br />


[![Dependabot compatibility
score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=aiohttp&package-manager=pip&previous-version=3.8.6&new-version=3.9.0)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores)

Dependabot will resolve any conflicts with this PR as long as you don't
alter it yourself. You can also trigger a rebase manually by commenting
`@dependabot rebase`.

[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)

---

<details>
<summary>Dependabot commands and options</summary>
<br />

You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits
that have been made to it
- `@dependabot merge` will merge this PR after your CI passes on it
- `@dependabot squash and merge` will squash and merge this PR after
your CI passes on it
- `@dependabot cancel merge` will cancel a previously requested merge
and block automerging
- `@dependabot reopen` will reopen this PR if it is closed
- `@dependabot close` will close this PR and stop Dependabot recreating
it. You can achieve the same result by closing it manually
- `@dependabot show <dependency name> ignore conditions` will show all
of the ignore conditions of the specified dependency
- `@dependabot ignore this major version` will close this PR and stop
Dependabot creating any more for this major version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this minor version` will close this PR and stop
Dependabot creating any more for this minor version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this dependency` will close this PR and stop
Dependabot creating any more for this dependency (unless you reopen the
PR or upgrade to it yourself)
You can disable automated security fix PRs for this repo from the
[Security Alerts page](https://github.com/pagopa/io-app/network/alerts).

</details>

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Cristiano Tofani <cri.tofani@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants