corona-warn-app / cwa-documentation

Project overview, general documentation, and white papers. The CWA development ends on May 31, 2023. You still can warn other users until April 30, 2023. More information:
https://coronawarn.app/en/faq/#ramp_down
Apache License 2.0
3.28k stars 346 forks source link

Missing capitalization for some sentences in the "Certificates" area #715

Closed jkrwdf closed 2 years ago

jkrwdf commented 3 years ago

Avoid duplicates

Technical details

Describe the bug

There is an inconsistent use of capitalization or non-capitalization of the first word in some standalone-sentences in the "Certificates" area. In the screenshots below, I demonstrate the cases which I consider incorrect and compare them to similar cases in other areas of the CWA and of CovPass as reference.

Steps to reproduce the issue

This screen shows the result of a tap onto a QR code in the "Certificates" area to show the persisted certificates of the person.

image

I claim that all the marked letters should be UPPER-case and it should be:

You may say that this is just one opinion and that the design team of CWA decided differently.

However, I can demonstrate that other locations of CWA do upper casing for comparable items.

For example, in the details of a single certificate, we find this:

image

Here we have "Gültig bis ..."

Or here, a quick-test result:

image

"Durchgeführt am ..." with upper case.

If we look at our "competition" in CovPass, we find:

image

"Geimpft am ..." with upper case.

Expected behaviour

Consistent use of upper-case sentence start for all standalone-sentences.

To explain what I mean with "Standalone"-sentence, here an example where the use of lower-case is accepted:

image

The "geboren" is here a direct addition to my name, separated by a comma.

Or here

image

where we also have a single sentence, just broken up to multiple lines: "Bis gestern ... bestätigte Neuinfektionen ...".


Internal Tracking ID: EXPOSUREAPP-9781

dsarkar commented 3 years ago

Hi @jkrwdf Thanks for the report. Internal Tracking ID: EXPOSUREAPP-9781. Best, DS


Corona-Warn-App Open Source Team

Ein-Tim commented 3 years ago

iOS PRs:

Ein-Tim commented 3 years ago

Android PRs:

Ein-Tim commented 3 years ago

I can confirm that version 2.13 has fixed this issue.

Here is a screenshot of the Corona-Warn-App version 2.13.1 running on an iPhone 6s with iOS 15.1.

As you can see, version 2.13 changed all the incorrect lower case letters to upper case.

jkrwdf commented 3 years ago

Thanks for the preview.

As Android user, I will have to wait a couple of days for review on my platform, I will close the issue thereafter.

jkrwdf commented 3 years ago

Hi,

CWA 2.13.2 for Android just arrived on my Google Pixel 3 XL, and unfortunately I have to kindly ask for rework here.

Issue 1

The "Letzte Impfung vor..." for Android is still lower case:

image

(for iOS, @Ein-Tim demonstrated above that it has been capitalized).

Issue 2

The only location where a lower-case "geboren" was valid, the RAT test details, has now also been capitalized:

image

I explicitly mentioned this occurrence of a valid lower capitalization already in my initial issue description. Search for this text: "To explain what I mean with "Standalone"-sentence, here an example where the use of lower-case is accepted:"

I therefore feel not guilty for this "excess", but ask you to revert this (single) one.

jkrwdf commented 2 years ago

Hi,

yesterday I had the chance to add a fresh vaccination to the CWA, and I see there that for the special case "Letzte Impfung heute" the "L" has been successfully capitalized, while next days "letzte Impfung vor 1 Tag" still is lower case. The same applies to the then final "letzte Impfung vor ... Tagen".

See the green and red marks in the following screenshot.

image

Give me an "L"...

jkrwdf

dsarkar commented 2 years ago

@Ein-Tim Thanks, is that the only missing capitalisation you found? We will re-open the internal ticket.

Ein-Tim commented 2 years ago

(@dsarkar It wasn't me 😉 It was @jkrwdf)

jkrwdf commented 2 years ago

Hello @dsarkar ,

this is @jkrwdf speaking, the other one with the six letter user name.

No, further missing capitalizations I did not see.

In https://github.com/corona-warn-app/cwa-documentation/issues/715#issuecomment-962505875 I annotated that due to the capitalization of "Geboren" for the start-of-line usages, the "geboren" in the in-line-usage for test results has now also been capitalized (likely those two UI locations used the same text-symbol reference) and that this one should be lower-cased again. I leave it to the developers assessment whether this was intentional or a side-effect and will not make my issue closing depending on it.

Best, @jkrwdf

Ein-Tim commented 2 years ago

PR: https://github.com/corona-warn-app/cwa-app-android/pull/4466

dsarkar commented 2 years ago

Hi @jkrwdf, thanks for your feedback, very much appreciated! Best, DS

jkrwdf commented 2 years ago

Closing after review in CWA 2.15.1