Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Patch fixes
- Inconsistency between incorrect classy and correct seven (tablesort-indicator.html.twig)
- Fixes incorrect capitalisation of very short words (requires to be ucfirst)
Beta Evaluation
Markup is "unfrozen".
Comment | File | Size | Author |
---|---|---|---|
#28 | Issue-2560049-by-hass-Incorrect-capitalisation-of-tr.patch | 43.49 KB | hass |
Comments
Comment #2
hass CreditAttribution: hass commentedIs there a tag for translation deadline?
Comment #3
hass CreditAttribution: hass commentedComment #4
hass CreditAttribution: hass commentedComment #5
hass CreditAttribution: hass commentedComment #6
hass CreditAttribution: hass commentedComment #7
hass CreditAttribution: hass commentedComment #8
cilefen CreditAttribution: cilefen commentedComment #9
hass CreditAttribution: hass commentedComment #10
chrisfree CreditAttribution: chrisfree at Chromatic commentedPatch in comment #2 looked good to me, but I found two instances in the documentation for pager.html.twig that also needed to be updated for consistency. The attached patch fixes those as well.
Hat tip to @mortendk for getting me motivated during his DrupalCon Barcelona session to start helping out with Classy!
Comment #11
hass CreditAttribution: hass commentedComment #12
alexpottNeeds a reroll
Comment #13
hass CreditAttribution: hass commentedOk, one file has already been fixed in an other case, so just removed this stuff now. Waiting for green light.
Comment #14
hass CreditAttribution: hass commentedComment #15
cilefen CreditAttribution: cilefen commented#2573437: Paging links are lowercased by default
Comment #16
hass CreditAttribution: hass commentedPer https://www.drupal.org/node/2573437#comment-10402251 we also have these strings in Views installation files. Let's fix them with the patch, too.
Comment #17
hass CreditAttribution: hass commented@crissfree: in what files are these config strings?
Comment #18
chrisfree CreditAttribution: chrisfree at Chromatic commentedLet me dig them up -- give me a few minutes.
Comment #19
chrisfree CreditAttribution: chrisfree at Chromatic commentedThere are quite a few places that this would need to be updated. I've included a list of all occurrences of "previous:" within yaml files below for a clearer picture.
A quick example would be the front page view that is installed with the standard site profile. This file can be found at:
/core/modules/node/config/optional/views.view.frontpage.yml
. The lines in question would be:which would become:
Here are all the instances of "previous: " within yaml files in D8 core:
I'll he happy to fix this up and patch this afternoon if this makes sense to move forward with.
Comment #20
hass CreditAttribution: hass commentedComment #21
hass CreditAttribution: hass commentedThanks chriss for noting this. A search yielded a lot of missing places with bugs. Please RTBC this patch asap. It's simple to review and it is RTBC from my side.
Comment #22
Wim LeersLooks great, with just one remaining problem/inconsistency.
Also: where is it documented that we require the first letter to be capitalized? I didn't realize this was a thing!
The Dutch translations here remain uncapitalized.
Yet here they became capitalized.
Which is correct?
Comment #23
hass CreditAttribution: hass commentedComment #24
hass CreditAttribution: hass commentedI guess the ucfirst is the right one. Damn oversight. The lowercase strings just create the same bugs in nl. We made the same faults in german as we don't know if lowercase is required.
Comment #25
hass CreditAttribution: hass commentedFixed the inconsistency.
Comment #28
hass CreditAttribution: hass commentedComment #29
Gábor HojtsyThe patch makes the same changes very consistently. #421118: [Meta] Standardize capitalization on actions has all kinds of people looking into it, so it seems like this is indeed a movement agreed on :)
Comment #30
xjmAh, this makes me happy. :) It brings these strings in line with the other actions we made sentence-cased already. Nice work on this issue! Committed and pushed to 8.0.x.