Content view type filter should be labeled "Content Type" for consistency

Translated strings like this cannot change until the next feature release so I am bumping this to 8.1.x.

I was trying to find an easy bug to begin contributing and found this.
Could you tell me how do I submit a patch for 8.1.x ? On git it seems the 8.1.x branch is 15 months old

@AbdealiJK That is a good question. A small patch like this one could be made against 8.0.x and will probably still work later. It never hurts to get a patch in, even if all it does is form the base for other work later.

Core contribution mentoring office hours are today, if you have the time.

I was applying patch but unable to find core/modules/node/config/optional directory.
pwd : /var/www/d8.1/core/modules/node/config
command apply : git apply content-type-label-2591357-6.patch

error: core/modules/node/config/optional/views.view.content.yml: No such file or directory

@snehi Patches are created from the root directory, so you should be in /var/www/d8.1/ when you apply the patch.

@cilefen done the same and that is the latest clone from

git clone --branch 8.1.x http://git.drupal.org/project/drupal.git d8.1

Branch 8.1.x isn't really set up yet. I think this issue is supposed to be on 8.1.x because of translatable text. If that's the case, it would be best to wait until that branch is ready to go.

I've managed to test the patch against 8.1.x branch. It works as it should, so I put the issue in RTBC.

+++ b/core/modules/node/config/optional/views.view.content.yml
@@ -421,7 +421,7 @@ display:
+            label: Content Type

The Content Type should be wrapped in single quotes, like so: 'Content Type'.

It is tricky to work out if this should have an upgrade path. On one hand this is in an obvious admin UI improvement, on the other, sites own their admin UI and this is not fixing something that is broken - plus they might have already translated this. As this is the first patch to change default translatable config I'm going to ping @GaborHojtsy for an opinion about the effect of changing this value.

I agree we should not do an upgrade path, this is site owned configuration and a usability improvement, not a bugfix.

A. For new sites, localize.drupal.org will recognize this new string in the config and Drupal will save its translation to the config translation to the right place.
B. For sites already using the old configuration but updating to the new Drupal version, the result *depends* on what exactly is the status of translation there and what happens next.

B1. If the string was not yet translated, then config translation does not yet contain the key. Although the active config will contain the old string 'Type', the translation will look up the new 'Content Type' source string, because the default config is the reference, and the default config was changed with the update.

B2. If the string is already translated and its the same as the shipped translation (not customized), then the question is what service kicks in first I think. If the config translation is saved again first, then its saved as customized and kept as-is. If localization update is ran first, then the localization will be updated from the translation of 'Type' to the translation of 'Content Type'. Again because the default configuration is kept as reference. I am not sure either service runs automatically with/after updates.

B3. If the string is already translated and its customized (different from the shipped translation), then all will be kept as-is, those string are protected by default.

#2428045: Comparing active storage to install storage is problematic, install storage may change anytime would of course make most of the special cases go away since locale would not compare to 'Content Type' if the active config was originally installed with 'Type'.

Changing with 'Content Type'.

Thanks everyone. I agree that we should probably not provide an upgrade path for this.

According to our content standards, I think this should be sentence-case ("Content type").

Can we get a screenshot of this patch's effect in the Views UI, rather than a screenshot of the patch being applied?

Thanks for proposing improvements to our UI text, and thanks @Gábor Hojtsy for explaining the impact on translations.

The patch tries to unify the labeling of content types in the content admin view. Prior to the patch:

It already is title cased in the column label, but the CSS styling makes it not apparent. Maybe we want to fix that too then?

Yes, I think we should potentially fix both places the title casing is used (might be a separate issue unless we rescope this to "Standardize on 'Content type' always for content types in UI text" or such):

[mandelbrot:drupal | Tue 15:46:42] $ grep -r "Content type" * | grep -v "vendor" | wc -l
      40
[mandelbrot:drupal | Tue 15:47:22] $ grep -r "Content Type" * | grep -v "vendor" | wc -l
       2

Thanks @Gábor Hojtsy.

I have solved the main issue and also have added these changes to the patch:

[mandelbrot:drupal | Tue 15:47:22] $ grep -r "Content Type" * | grep -v "vendor" | wc -l
2

Verified the patch "2591357-21.patch" mentioned in #21.
Steps performed to verify the patch:
1. Navigate to http://localhost/drupal/admin/content.
2. Verify the filter name "Type" before and after patch is applied.
3. As the Filter name has not been updated after applying the patch, its Retest Failed.

So, issue can be marked as "RETEST FAILED".

Still missing the single quotes as pointed out in #15.

Update the patch for introduce the single quotes as pointed out in #15.

Don't we want to add an update path and so adapt the existing configuration installed on sites?

Testing the patch, will update soon.

Hey the patch works perfectly fine. Below are the steps followed:

1. Took the git clone for 8.1.x.
2. Applied the patch with git apply -v
3. Reviewed content page with admin login, the results were seen as attached in the screenshot.

I also tested the same. Worked for me. Thanks.
making it RTBC.

IMHO we still want an update path, see #26

Fixing tags. Adding back needs upgrade path. Not sure what would be the best tag for DCAsia, certainly not with a +. Removing that.

+++ b/core/modules/views/views.install
@@ -361,5 +361,19 @@ function views_update_8005() {
+  $displays = $view->get('display');
+
+  $displays['default']['display_options']['filters']['type']['expose']['label'] = 'Content type';

That display may or may not be there, the filter may not be there, it may not be exposed, etc. It may not have the original label either (eg. on a multilingual site it is very likely it would not be the old one anymore). So just plainly updating it without consideration if the view existed, the filter existed, the label was still the old one etc. is not very good I am afraid.

Thanks for review, Gábor. Have updated the patch.

Superb, thanks for the quick fix. Now only needs tests for the upgrade path :)

Added tests for the upgrade path and updated the patch a bit, as we forgot to update Type field label.

I think that is plenty of coverage for a small label change thanks a lot :)

Sorry I agree with #15, #16 and #18 that we don't want an upgrade path for this. It does no harm to leave things the same on sites that already have this config.

Went ahead and committed/pushed #25 to 8.1.x, thanks!

Works for me :) Thanks!