Problem/Motivation

Views had a CSS file inside the module code that force lowercase of translatable strings. This did not let some translations that are upper case first like German, French and others, be correct in their language. The forcing of lowercase with CSS needed to be removed to allow translators to write uppercase first.

Proposed resolution

D7 views module (following up)

(already committed to views 7.x) removed the css that lowercased things.
patch in #34 #70 is for views 7.x:
It keeps the translations valid by returning strings to uppercase, but introduces small UX difference by having some English operations use Capitalized action words.

D8 (fixed)

remove the css that lowercased things. (patch in #38 is for drupal core 8.x)

Before / After in German D8 screenshots

Without patch:
Without patch in German
With patch:
With patch in German

Remaining tasks

  • up-to-date before and after screenshots in english for D7 views module (to clarify the small UI inconsistency going to be introduced)
  • up-to-date before and after screenshots using a translation for D7 views module (to clarify that the translations are broken before the patch, but ok after the patch).

User interface changes

Yes. Case of things.

API changes

No API changes.

Background about commits

#421118: [Meta] Standardize capitalization on actions explains the reasons behind again and shows us that contrib has changed everywhere in D7 times.

Patch in #0 was the correct one for D7, but dawehner has committed patch in#10 - that lowercases the source strings and broke existing strings. This needs to be rolled back with #34 in views D7 so we end up with the correct patch in #0.

Original report by hass:

Patch attached.

With the patch all the currently broken translatable strings in dropdown menus revert to correctly capitalization - their capitalization is kept how they are in the translatable strings.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

hass’s picture

FileSize
17.95 KB

Screenshot how it looks correctly with the patch (German translation)

ViewsUI.png

dawehner’s picture

You know the original strings are also not lowercase, this was really part of the initial design. see

      $rearrange_text = t('And/Or, Rearrange');

for example, so this is more or less a nogo.

hass’s picture

I don't understand you... The bug here is that the module currently forces all properly capitalized translatable strings to *lowercase*. This is wrong behavior!

merlinofchaos’s picture

The UX pattern has been that actions, when they are standalone, are in lowercase. Please feel free to argue with UX folks about that.

hass’s picture

Than you need to change the strings inside t(), not make them lowercase with css. If case in t() is kept as is we don't have a problem. Lowercase first of Edit links is for sure minor as it's still proper English where all strings lowercase is not correct. There i no need to discuss with any UX guy.

tim.plunkett’s picture

Category: bug » feature
Status: Needs review » Closed (won't fix)

You can add text-transform: none; to your admin theme's CSS if you like.

hass’s picture

Category: feature » bug
Status: Closed (won't fix) » Needs review

It sound like you don't understand that the current is wrong. If you like You can add the lowercase css to your theme. Here we need to make it correct for *all* themes and translations in this world by applying the above patch.

tim.plunkett’s picture

Title: Remove capitalization abuse in translatable strings » Remove capitalization abuse in strings

This has nothing to do with translations, just UI strings. It affects the untranslated strings exactly the same.

hass’s picture

I'm sure an english only developer think it's wrong capitalization in english and just added this wrong css line. I know how often we discussed such type of issues in views queue in last years.

merlinofchaos’s picture

hass: You are not the arbiter of what is right and wrong. You are not the person who gets to DEMAND we fix stuff because of your opinion about right and wrong. It is expected that you make your case and provide reasoned arguments and work WITH us to make positive changes.

Your method of dealing with this issue queue is negative in the extreme. It would be beneficial to the entire community if you would modify your behavior to result in more positive interactions. As it is, the entire Views team is currently frustrated with you.

Now. This is my final word: This can be discussed with the UX folks who made that decision. I recognize that due to translation it is not ideal. However, your solution makes a UX change without getting input from the people who wanted the UX to be that way, and demands that we make that change for you.

Please stop trying to force it on us.

hass’s picture

With 5 years expierience in translatability reviews I know that the current IS wrong. I cannot translate views correctly to German and all other languages correctly. If above words are lowercase in German, it IS wrong. Nothing to prove, I speak my native language very well or can anyone prove the oposite?

I provided a patch, a screenshot and information. Nothing can be more positive. The negative would be - just to complain and telling me that I'm wrong.

I have no idea what you think what the UX folks have decided, but it may be a wrong understanding of their suggestions or the implementation is just wrong. As said, if you need the strings lowercase in english, all english strings need to change to lowercase, but this way translators can write uppercase first; if required.

I'm sorry that you feel bad and understand my intention wrong, but being more open minded to expierenced translators can help a lot and would frustrate me not, helping with more patches for bugs in views.

dawehner’s picture

FileSize
1.75 KB

You have a fight of ux vs. translatability vs. string freeze. Here is a patch which changes the string freeze.

tim.plunkett’s picture

Version: 7.x-3.3 » 7.x-3.x-dev

Changing version for the testbot.

Status: Needs review » Needs work

The last submitted patch, 1496418.patch, failed testing.

tim.plunkett’s picture

Status: Needs work » Needs review

#12: 1496418.patch queued for re-testing.

tim.plunkett’s picture

Category: bug » feature
Status: Needs review » Closed (won't fix)

Closing.

hass’s picture

Category: feature » bug
Status: Closed (won't fix) » Needs review

The bug is not fixed as I know.

dawehner’s picture

But neither has someone reviewed the latest patch. You could have done this during reopening the issue.

Please be constructive, so we can achieve something at the end of the day.

webflo’s picture

FileSize
2.32 KB

I think the title attributes has to be lowercase too.

dawehner’s picture

From my perspective this patch looks fine, though i would like to have some feedback of hass first.

hass’s picture

I think it's incorrect that all strings in t() become lowercase as it causes translation troubles, but the css lowercase is plain wrong and all languages except english. It's not wrong to have the strings uppercase first in english, but I can accept this change as it hopefully allows us to fix the casing in German. The string changes may add new context sensitive issues, but its far better than today.

hass’s picture

Status: Needs review » Reviewed & tested by the community

With my patch in #0 the string freeze is not affected at all. #19 is in general the wrong approach as we need to write everything Uppercase first in German and in English ucfirst it is also correct. However I don't fight this ucfirst bugs in core here.

I'm also fine if you'd like to break strings with patch in #19, but don't wait any longer please as we can fix this string break with minutes after a release on localize.d.o site and we do not have static PO files any longer. These t() bugs need to be fixed for D8 as these strings and css hack is not core worthy.

dawehner’s picture

Version: 7.x-3.x-dev » 8.x-3.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)
Issue tags: +Novice, +VDC

I disagree with changing the visual output of views by default, you know people made the decision to keep the words in lowercase
so we should respect that.

Committed the patch in #19
The patch itself need some easy rerolling in d7 as css files are gone/and a lot of variables changed.

hass’s picture

The patch itself need some easy rerolling in d7 as css files are gone/and a lot of variables changed.

-> means D8

thx.

xjm’s picture

This patch is doing the opposite of what core is doing in D8:
#421118: [Meta] Standardize capitalization on actions

So I think we should revert the case change.

The CSS however is unneeded anyway IMO.

hass’s picture

LOL, thanks for sharing this link. ***G***

dawehner’s picture

Well for d7 i would like not to change anything, but for drupal8 we could just commited the patch in #0

hass’s picture

You could save translators time and rollback and commit #0 to D7, too. Than we do not need to translate twice what suxxx a lot. The translation problems are the same in D7 and there are many contrib modules that are already fixed. We can mark the linked case as need backport to D7, too.

xjm’s picture

Version: 8.x-3.x-dev » 7.x-3.x-dev
Status: Patch (to be ported) » Fixed

I agree that #0 is the correct fix for D8. Committed here to 8.x-3.x:
http://drupalcode.org/project/views.git/commit/9e3174e

Marking fixed per #27, though my personal opinion is that this patch should not have gone into D7 at all, since it involves both a UI change and a string change. But that's the core-brain talking. :) Edit: Especially since one can override CSS with, you know. The theme layer.

dawehner’s picture

@xjm
As the output itself didn't changed for the user (it is still the lowercase), this is just a string change, but if the folks who
are actually victims of string changes agree, well...

hass’s picture

Status: Fixed » Needs work

As Dave said, contrib modules should change in D7, too. Let's rollback the translatable string (lowercasing) changes in D7, please. Translating twice for nothing is only a wast of time. Please safe limited translators time.

xjm’s picture

Re: #30 -- The output didn't change for the English strings, but it did change wherever they're translated. ;) Which is the point of the initial patch, to allow German to properly capitalize its Nouns or whatever, right? But it's still a minor UI change and a string change for something that already has a workaround. That's what I'd argue if it were core, anyway. :)

Re #31, @Dave Reid did not say anything about backporting that issue to Drupal 7. Core only breaks strings for D7 if there's a severe bug related to them, not for a UX nicety.

hass’s picture

My point is that all the previously used strings are already translated (properly capitalized). Now Views D7 breaks these strings and all view users will not have a translated UI. This patch should be rolled back to use the previously correct capitalized words. It's not a problem to capitalize these strings in English. Other contrib modules do it the same way already as Dave Reid noted (like D8). Views had all strings ready for D8, now the strings are broken in all translations and we need to translate them again (suxxx). As example there was never any module (and I mean *all* modules) that have used t('add') just as one example. Therefore the UI will not be translated in 3.6.

The CSS hack was really wrong but with the committed patch all has been made more bad than it need to be. I complained about this capitalization issue a lot and it looks like it requires a Dave Reid to get heard.

So, again - do the right thing and roll it back, *please*. Thanks for your understanding.

hass’s picture

Title: Views: Don't change capitalization of translatable strings with CSS » Remove capitalization abuse in strings
Project: Drupal core » Views (for Drupal 7)
Version: 8.x-dev » 7.x-3.x-dev
Component: views_ui.module » User interface
FileSize
1.98 KB

D7: As discussed, this is a rollback of the bad string change patch. The previously used CSS lowercase forcing is kept out as this is the wrong way to capitalize strings.

hass’s picture

Status: Needs work » Needs review

@Todo: review if this is still in core or I have only seen it just on an outdated D8 demo site http://sc1c414b09eaec05.s2.simplytest.me/admin/structure/views/view/arch...

.dropbutton, .dropbutton-wrapper input {
    text-transform: lowercase;
}
dawehner’s picture

Add a tag.

hass’s picture

Issue summary: View changes

Added issues summary

hass’s picture

Added

hass’s picture

Project: Views (for Drupal 7) » Drupal core
Version: 7.x-3.x-dev » 8.x-dev
Component: User interface » views_ui.module
FileSize
675 bytes

D8 patch, as root cause bug is not fixed in core, too.

hass’s picture

Title: Remove capitalization abuse in strings » Don't change capitalization of translatable strings with CSS
Project: Views (for Drupal 7) » Drupal core
Version: 7.x-3.x-dev » 8.x-dev
Component: User interface » views_ui.module
hass’s picture

Title: Don't change capitalization of translatable strings with CSS » Views: Don't change capitalization of translatable strings with CSS
hass’s picture

RTBC

hass’s picture

FileSize
8.97 KB

Now capitalization of operation links is correct:
2013-02-11_122245.png

YesCT’s picture

So is both #34 and #38 needed for D8?

Or is just #38 needed for D8?

This issue is pretty confusing, and the issue summary might need updating, with a D7 section and a D8 section.

hass’s picture

Title: Remove capitalization abuse in strings » Views: Don't change capitalization of translatable strings with CSS
  • #34: D7, views rollback patch for faulty commit (needs commit BEFORE next views release, independent of D8 patch commit), RTBC.
  • #38: D8, core patch, RTBC.
hass’s picture

@YesCT: Can you set #38 RTBC, please?

Kars-T’s picture

Status: Needs review » Reviewed & tested by the community

Patch in #38 works for me and fixes the lowercase issue.

YesCT’s picture

Issue tags: +Needs screenshots

code in #38 looks good. I did not check it manually, but #46 sounds like it did.

Making this change for 8.x core makes sense to me. rtbc from me too.

I think up-to-date before and after screenshots in english (and german) would help (and be great to also add to the issue summary).
Updating the issue summary will also help (to clarify that the patch in #38 is all that is needed for d8). Actually, I'll update the summary.

YesCT’s picture

Project: Drupal core » Views (for Drupal 7)
Issue summary: View changes

Updated issue summary.

YesCT’s picture

Project: Views (for Drupal 7) » Drupal core
Kars-T’s picture

Issue tags: -Needs screenshots
FileSize
16.72 KB
27.78 KB

Added screenshots

YesCT’s picture

Awesome! That was fast.
For ease of review, can you embed those in the issue summary? (or in the comment)
I like to use dreditor to make embedding easier. :)

YesCT’s picture

Project: Drupal core » Views (for Drupal 7)
Issue summary: View changes

clarified what needs to be done for 8.x

Kars-T’s picture

Project: Views (for Drupal 7) » Drupal core

Added screenshots in German to summary.

hass’s picture

#1 also has a screenshot that show how it looks with the fix...

YesCT’s picture

@hass, yes, but core changes so fast, a recent ones are helpful. :)

hass’s picture

Project: Drupal core » Views (for Drupal 7)
Issue summary: View changes

Updated summary with screenshots in German language.

hass’s picture

Project: Views (for Drupal 7) » Drupal core

There are no changes. Broken from the first day and not changed since years. :-( Added to summary

Dries’s picture

Version: 8.x-dev » 7.x-dev

Committed to 8.x. Thanks. Moving to 7.x for #34.

hass’s picture

Project: Drupal core » Views (for Drupal 7)
Version: 7.x-dev » 7.x-3.x-dev
Component: views_ui.module » Code
hass’s picture

Component: Code » User interface
tim.plunkett’s picture

Status: Reviewed & tested by the community » Needs review

I'm not sure we want to make this change at this point in D7. Either way, it needs to be rerolled.

hass’s picture

It was rerolled 4 days ago.

hass’s picture

hass’s picture

Priority: Normal » Critical

@tim: dawehner has broken these strings I'm fixing here by committing the broken patch in #19. The current strings in DEV are not translated in any language. With the rollback we revert the lowercase bug introduced by #19.

We want to rollback for sure and before the next views release! Critical for next release as it breaks translated strings in all languages.

Xano’s picture

Priority: Critical » Normal

@hass, calm down. The tone of your comments is counter-productive. This issue is not major (breaking translations does not cause the module to be broken completely, especially if this happens on admin pages only), and it is more complicated than you seem to realize: Removing the CSS keeps translations intact, but causes the original strings to be incompatible with UX guidelines. Making the original English strings comply with the guidelines breaks existing translations.

We know what's wrong here. The correct approach has to be decided on together.

Seeing as the patch from #19 only changes three administrative strings *and* it actually makes the strings comply with the standards, I'm inclined to say that patch was correct.

dawehner’s picture

Priority: Normal » Critical
FileSize
2.32 KB

Regarding the core patch: Are we actually sure this should have went in? Just checked the following pages admin/content admin/structure/taxonomy admin/structure/taxonomy/menu etc. and only admin/content and now views uses upper-case.

This is what I think is a useful patch, revert the previous state. I'm sorry but sometimes "platzt einem einfach der Kragen. Das Ziel ist doch konstruktiv etwas beizutragen und sich nicht gegenseitig wegen Lappalien fertigzumachen.

YesCT’s picture

Wait, it was commited to 8.x, it removed the css that lowercased it.

And we'll fix the actual strings (which are going uppercase as part of #421118: [Meta] Standardize capitalization on actions).

OR

@dawehner are you worried about the 7.x commit that was in #23 committing the patch from #19?

I'm having a hard time following what happened in 7.x.

-----
In general, I agree that 7.x needs a careful careful look by calm people cooperating.

hass’s picture

#63 is NOT a correct patch. Remove the css lowercasing, please. It's wrong in all languages except english. I don't get what is so difficult to understand about this. Don't argument with wrong UI guidelines, please. #421118: [Meta] Standardize capitalization on actions is the correction to this wrong "guideline".

Let's stop this now, do the right thing and commit my patch and we are all done and don't need to waste our valuable time with needless discussions and over a timeframe of 1 year. Heck, why is this so difficult to accomplish?

hass’s picture

@Xano: I'm really tired of discussing faults of past where it seems to require a Dave Reid to get something fixed that I said, too and is still correct. Waste of time only!

Xano’s picture

Are we actually sure this should have went in?

Yes. At least the CSS lowercasing strings was wrong. We can, however, discuss what to do with the existing strings themselves.

It's wrong in all languages except english.

No, it is not. In Dutch it is perfectly fine to write single words or phrases without capitalization, if the words themselves do not require it (e.g. the words are not names).

dawehner’s picture

Issue tags: -VDC

Removing VDC tag for now.

Xano’s picture

Drupal 8

See \Drupal\views_ui\ViewEditFormController\getFormBucket(). The labels are correctly capitalized as per #421118: [Meta] Standardize capitalization on actions in Drupal 8. There is also no CSS that lowercases test on Views administration pages.

This means that the issue in Drupal 8 is fixed, and the issue is indeed not about VDC anymore.

Drupal 7

The CSS to lowercase operations is gone. Operations strings have been converted to lowercase as per the guidelines. This means that Views 7.x-3.x now complies with the UX guidelines and that translations are now lowercased anymore. The only problem at this moment is that translations for three original strings are now broken, because the originals changed.
The question is: do we want to keep Views 7.x-3.x compliant and break three translations, or do we keep the translations despite a small UX bug?

dawehner’s picture

Priority: Critical » Minor
FileSize
1.96 KB

Well UI/string changes seems to be okay according to hass, so posting a patch, as there is no other available for D7.

Xano’s picture

Status: Needs review » Reviewed & tested by the community

Let's go with the patch from #70 as it causes the least regression. It looks good, so RTBC'ing it. Let's wait for the test results to come back before committing the patch, though.

YesCT’s picture

Clarification question:
The patch in #70 is the last proposal in #69?

we keep the translations despite a small UX bug

Xano’s picture

Yes.

Xano’s picture

Issue summary: View changes

a

YesCT’s picture

Issue summary: View changes

Updated issue summary, to clarify strategy for 7.x and update remaining tasks.

YesCT’s picture

I updated issue summary to clarify the D7 approach, and update remaining tasks (which views maintainer might not need, but I thought they would help).

dawehner’s picture

Status: Reviewed & tested by the community » Fixed

Puh, finally that's over.

Automatically closed -- issue fixed for 2 weeks with no activity.

Anonymous’s picture

Issue summary: View changes

Updated issue summary, took out screenshot of patch #0. as it lacks content in the summary, and we want current screenshots there.