Problem/Motivation

Cleanup the content management user experience.
The management of the visibility of the author and date information is living in the content type configuration page instead of the Manage display tab.

Proposed resolution

Show the author and creation date on the manage display tabs.
Remove the display author information on the content type settings page.
Remove the theme settings "show author picture on the nodes".

Remaining tasks

Show the author and creation date on the manage display tabs.
Remove the display author information on the content type settings page.
Remove the theme settings "User pictures in posts".
Handle the upgrade path.

User interface changes

Disappearance of the vertical tab "Display settings" on the content type edit page.
Disappearance of the theme setting "User pictures in posts"

API changes

None.

Data model changes

Probably none.

CommentFileSizeAuthor
#129 interdiff-1399990-127-129.txt1.63 KBYurkinPark
#129 core-remove_display_author-1399990-129.patch47.27 KBYurkinPark
#127 interdiff-1399990-124-127.txt7.44 KBYurkinPark
#127 core-remove_display_author-1399990-127.patch45.29 KBYurkinPark
#124 interdiff-1399990-120-124.txt616 bytesYurkinPark
#124 core-remove_display_author-1399990-124.patch37.79 KBYurkinPark
#120 interdiff-115-120.txt11.62 KBilya.no
#120 core-remove_display_author-1399990-120.patch37.42 KBilya.no
#115 interdiff-112-115.txt4.57 KBsorlov
#115 core-remove_display_author-1399990-115.patch37.58 KBsorlov
#114 core-remove_display_author-1399990-114.patch37.63 KBsorlov
#112 1399990-108-reroll.patch36.81 KBandypost
#110 interdiff.txt4.32 KBandersen_ti
#110 core-remove_display_author-1399990-110.patch37.17 KBandersen_ti
#109 2019-11-15_12-36.png2.24 KBdavidferlay
#109 2019-11-15_12-26.png13.57 KBdavidferlay
#109 2019-11-15_12-25.png30.61 KBdavidferlay
#109 2019-11-15_12-31.png7.11 KBdavidferlay
#108 interdiff-97-108.txt17.64 KBilya.no
#108 core-remove_display_author-1399990-108.patch36.57 KBilya.no
#105 interdiff-97-105.txt17.71 KBilya.no
#105 core-remove_display_author-1399990-105.patch35.49 KBilya.no
#97 core-remove_display_author-1399990-97.8.6.x.patch36.19 KBartusamak
#92 interdiff.txt1.2 KBk4v
#92 1399990-91.patch42.24 KBk4v
#90 1399990-90.patch41.04 KBk4v
#81 1399990-81.patch42.03 KBlplp
#81 interdiff-1399990-79-81.txt654 byteslplp
#79 1399990-79.patch41.63 KBlplp
#75 author-field.png14.26 KBifrik
#74 cap_submitted_date_options.png16.92 KBartusamak
#74 cap_submitted_author_options.png8.58 KBartusamak
#74 cap_submitted_new_page.png8.3 KBartusamak
#74 cap_submitted_new_article.png11.75 KBartusamak
#74 cap_submitted_display_page_old.png54.86 KBartusamak
#74 cap_submitted_display_article_old.png67.35 KBartusamak
#74 cap_submitted_display_page_new.png56.17 KBartusamak
#74 cap_submitted_display_article_new.png62.18 KBartusamak
#74 cap_submitted_content_type_edit_without.png33.26 KBartusamak
#74 cap_submitted_content_type_edit.png29.24 KBartusamak
#72 author-field.png13.86 KBifrik
#71 interdiff-1399990-68-71.txt8.47 KBartusamak
#71 core-remove_display_author-1399990-71.8.4.x.patch41.61 KBartusamak
#68 interdiff-1399990-67-68.txt761 bytesartusamak
#68 core-remove_display_author-1399990-68.8.4.x.patch41.51 KBartusamak
#67 interdiff-1399990-64-67.txt6.24 KBartusamak
#67 core-remove_display_author-1399990-67.8.4.x.patch40.87 KBartusamak
#64 interdiff-1399990-62-64.txt9.16 KBartusamak
#64 core-remove_display_author-1399990-64.8.4.x.patch40.89 KBartusamak
#63 interdiff-1399990-62-63.txt6.43 KBartusamak
#63 core-remove_display_author-1399990-63.8.4.x.patch41.94 KBartusamak
#62 interdiff-1399990-30-62.txt4.66 KBartusamak
#62 core-remove_display_author-1399990-62.8.4.x.patch35.13 KBartusamak
#60 core-remove_display_author-1399990-60.8.4.x.patch33.76 KBartusamak
#58 core-remove_display_author-1399990-58.8.4.x.patch62.42 KBartusamak
#55 core-remove_display_author-1399990-55.8.4.x.patch32.64 KBartusamak
#54 date-formats.png61.87 KByoroy
#54 submitted-by.png24.14 KByoroy
#52 core-remove_display_author-1399990-52.8.4.x.patch35.99 KBartusamak
#50 core-remove_display_author-1399990-49.8.4.x.patch36.03 KBartusamak
#46 core-remove_display_author-1399990-44.patch27.86 KBartusamak
#43 interdiff-1399990-40-43.txt2.04 KBartusamak
#43 core-remove_display_author-1399990-43.patch27.84 KBartusamak
#40 interdiff-1399990-38-40.txt553 bytesartusamak
#40 core-remove_display_author-1399990-40.patch25.54 KBartusamak
#38 interdiff-1399990-36-38.txt6.81 KBartusamak
#38 core-remove_display_author-1399990-38.patch24.92 KBartusamak
#36 interdiff-1399990-34-36.txt8.19 KBartusamak
#36 core-remove_display_author-1399990-36.patch19.36 KBartusamak
#34 core-remove_display_author-1399990-34.patch11.04 KBartusamak
#28 interdiff-1399990-26-28.txt5.26 KBartusamak
#28 core-display_author_date-1399990-28.patch23.48 KBartusamak
#26 interdiff-1399990-24-26.txt648 bytesartusamak
#26 core-display_author_date-1399990-26.patch18.25 KBartusamak
#24 interdiff-1399990-22-24.txt3.37 KBartusamak
#24 core-display_author_date-1399990-24.patch18.25 KBartusamak
#22 Sélection_004.png20.31 KBartusamak
#22 Sélection_003.png72.45 KBartusamak
#22 Sélection_002.png22.82 KBartusamak
#22 Sélection_001.png56.28 KBartusamak
#22 interdiff-1399990-19-22.txt8.73 KBartusamak
#22 core-display_author_date-1399990-22.patch15.77 KBartusamak
#19 interdiff-1399990-16-19.txt553 bytesartusamak
#19 core-display_author_date-1399990-19.patch16.95 KBartusamak
#16 core-display_author_date-1399990-16.patch16.34 KBartusamak
#16 display-config.png61.12 KBartusamak
#2 display_settings.png133.86 KBjenlampton
#16 display-edit.png52.91 KBartusamak
#16 display-content.png21.09 KBartusamak
Screen shot 2012-01-10 at 12.44.38 PM.png20.93 KBbstoppel

Comments

naught101’s picture

Issue tags: -Usability

might be easier to attack from this angle, since neither of those data are #1194470: Content type display: separate author and date information

jenlampton’s picture

Issue tags: +Usability
StatusFileSize
new133.86 KB

I completely agree with @bstoppel, there's really no reason for this information to be on the edit tab at all.

In addition, if we were to make the author and date information configurable via the manage display tab, then we would actually be able to allow people to choose which date format the date should use, instead of forcing the use of medium (which nobody knows about anyway).

I also think they should be separated on the manage display list - then we could also control the display of the author name, linked, or not linked? for example.

move information in the display settings vertical tab to the manage display tab

jenlampton’s picture

karschsp’s picture

Seems like this would be really easy/redundant if we could get this in: #1188394: Make title behave as a configurable, translatable field (again)

karschsp’s picture

Ugh, posted that to the wrong issue, but along those lines, should we make author and date information fields to help with this?

aaronbauman’s picture

One might even argue that having display fields that don't appear on the manage display tab is a UX bug...

dman’s picture

Issue tags: +Usability

+1.
Currently we still have to go and attack the tpl if we want to adjust the positioning of the publishing data for theming, eg (title, image, credits)
When everything else went fieldable, it now looks like this is a throwback. Also, being able to split, toggle & reorder the date and byline makes sense.
it feels like a pretty cheap win?

shp’s picture

+++ for moving this to "Manage display" tab. As well as extended configuring of this string.

jenlampton’s picture

Well actually, the submitted line is a little tricky, since it's translated, and generally inside the same HTML tag (in this case, a P tag). But, we could have a special template that handles all cases (remember when Drupal use to actually use theme_submitted()?).

Here's a look at some of the other work we are doing with node submitted info in templates, if anyone is curious: #1927584: Add support for the Twig {% trans %} tag extension

naught101’s picture

I'm wondering if there's any reason not to just treat the author and date as normal fields, for display purposes? Like views does (/can).

kclarkson’s picture

Please ohh Please Provide an Option to Turn author display off by default when creating new content.

Nothing more agitating then realizing that you forgot after you just created 10 content types :)

You could actually remove it all together if you wanted. Most sites I would say don't use the default author information.

yoroy’s picture

Version: 8.0.x-dev » 8.2.x-dev
Issue summary: View changes
joyceg’s picture

This is a valid argument. The form settings should be connected with the display settings.

yoroy’s picture

Issue tags: +DevDaysMilan

Worthy of an initial patch for sure.

artusamak’s picture

Assigned: Unassigned » artusamak

Let's have a look at this.

artusamak’s picture

Status: Active » Needs review
StatusFileSize
new61.12 KB
new21.09 KB
new52.91 KB
new16.34 KB

Here is a first shot at this.

Several questions are raised:

* What do we want to do with the methode getter displaySubmitted() and the associated setter setDisplaySubmitted() from the NodeType interface? I suggest to mark it as deprecated since it's "doesn't make sense anymore" if we consider the creation date and the author information as regular fields.
* Should we provide a default template for the author + creation date to let users that liked the "Submitted by..." information? Maybe be create a field formatter for the author name? A Twig block to extend?
* Should the author_picture theme setting be kept since it was only used in the node display submitted area? It's logic new implementation is in the configuration of the entity view mode used from the author formatter settings. Since it may introduce a backward compatibility issue, feedbacks are welcomed.
* The variable "author_attributes" is not injected anymore since it was displayed in the div wrapper around the "Submitted by..." content. Should we conserve this variable?

View mode configuration screen:
View mode configuration screen

Preview of the default output:
Defaut output preview

New edit screen for the content type:
New edit screen without the author display info

At least we have a starting point. :)

artusamak’s picture

Assigned: artusamak » Unassigned

Status: Needs review » Needs work

The last submitted patch, 16: core-display_author_date-1399990-16.patch, failed testing.

artusamak’s picture

Assigned: Unassigned » artusamak
Status: Needs work » Needs review
StatusFileSize
new16.95 KB
new553 bytes

Do not rely on the removed variable anymore. Generate the metatags everytimes.

artusamak’s picture

Assigned: artusamak » Unassigned

Status: Needs review » Needs work

The last submitted patch, 19: core-display_author_date-1399990-19.patch, failed testing.

artusamak’s picture

Issue summary: View changes
Status: Needs work » Needs review
StatusFileSize
new15.77 KB
new8.73 KB
new56.28 KB
new22.82 KB
new72.45 KB
new20.31 KB

In this new version of the patch, as shown below, I added a new extrafield to assure the backward compatibility.

Manage display new screen

It relies on the "Authored by" and "Authored on" configuration which lets the user customize the rendering of this piece of information. There is a dedicated template that reuses the previous structure.
Content output
If the user want to split the information, he just has to hide the extrafield and he will be able to place the "Authored by" and "Authored on" fields to two different places of the node.
Example illustrated below:

1) The extrafield is hidden.
Extrafield hidden

2) Content preview.
content preview

Status: Needs review » Needs work

The last submitted patch, 22: core-display_author_date-1399990-22.patch, failed testing.

artusamak’s picture

Status: Needs work » Needs review
StatusFileSize
new18.25 KB
new3.37 KB

Starting to fix some tests.

Status: Needs review » Needs work

The last submitted patch, 24: core-display_author_date-1399990-24.patch, failed testing.

artusamak’s picture

Status: Needs work » Needs review
StatusFileSize
new18.25 KB
new648 bytes

Removal of the hardcoded node bundle.

Status: Needs review » Needs work

The last submitted patch, 26: core-display_author_date-1399990-26.patch, failed testing.

artusamak’s picture

Status: Needs work » Needs review
StatusFileSize
new23.48 KB
new5.26 KB

Recheck against the tests.
Still a WIP.

kclarkson’s picture

really LOVING the work here!

Big props to @Artusamak

Status: Needs review » Needs work

The last submitted patch, 28: core-display_author_date-1399990-28.patch, failed testing.

artusamak’s picture

Issue summary: View changes
Status: Needs work » Needs review

This topics is bigger than it looks.
We need to agree on what we want before going further.

1) Display submitted flag
We have this flag: \Drupal\node\Entity\NodeType::$display_submitted that is used to show / hide the actual creation date and author of a node.
If we remove this flag, we switch to manipulate two base fields instead. It means that nodes will now always have at least two fields shown in the Manage displays / Manage form displays tabs.

Question 1: Do we want to keep this flag?

I think that we have to in order to avoid backward compatibility issues. Plus it would mean removing two methods from \Drupal\node\NodeTypeInterface (displaySubmitted() and setDisplaySubmitted()) which is not possible to 8.x. What we should do is to mark them as deprecated.

2) Extra field & upgrade path
Now that we have two base fields everywhere, how can we handle the upgrade path? People with an existing site displaying the author and creation date needs to still see those information when they update their sites.
An option is to create an extra field (as hook_entity_extra_field_info()) that still shows the "Submitted on Sun, 06/26/2016 - 11:33 by Artusamak" output.
In this situation the tricky question is how to live with both the extra field and the "Authored by" + "Authored on" base fields.
A trick could consist to render the "Authored by" and "Authored on" fields within the extra field, it would make it configurable for the date and author format and wouldn't render it twice. The drawback being if you hide the author and creation date, you can end up with a "Submitted on by" string.

Question 2 : How to handle the legacy situation?

Option 1: Create an extra field with an hardcoded format.
Option 2: Create an extra field that renders the authored on and authored by fields and have a fallback format if the fields are hidden.
Option 3: Explore another implementation on templates.

artusamak’s picture

Issue summary: View changes
bojanz’s picture

I agree with deprecating the methods and removing the settings.
The simplest solution for preserving the previous look would be to modify the node template / preprocess hook, there's probably no need for an extra field.

Thanks for driving this forward, Artusamak!

artusamak’s picture

StatusFileSize
new11.04 KB

Retesting this template approach.
This is a WIP.

Status: Needs review » Needs work

The last submitted patch, 34: core-remove_display_author-1399990-34.patch, failed testing.

artusamak’s picture

Status: Needs work » Needs review
StatusFileSize
new19.36 KB
new8.19 KB

Formatters settings have been added for the AuthorFormatter and TimestampFormatter.php formatters in order to let users show / hide the "submitted by" and "on " strings around the field values. This may be relocated to the tpl files of the fields if it makes more sense.

We should have improvement on the tests results related to the content creation.
This is still a WIP.

Status: Needs review » Needs work

The last submitted patch, 36: core-remove_display_author-1399990-36.patch, failed testing.

artusamak’s picture

Status: Needs work » Needs review
StatusFileSize
new24.92 KB
new6.81 KB

Well this is going forward. :)
Let's see what the bot says. It's still a WIP.

Status: Needs review » Needs work

The last submitted patch, 38: core-remove_display_author-1399990-38.patch, failed testing.

artusamak’s picture

Status: Needs work » Needs review
StatusFileSize
new25.54 KB
new553 bytes

Fixing one more test.
The RDF errors are... tough to understand and debug.

naught101’s picture

@artusamak: maybe you should set up a local testing environment, and use it instead of the test bot? That way people are just notified of important patches (e.g. working patches, or patches where you need feedback on some specific component).

Status: Needs review » Needs work

The last submitted patch, 40: core-remove_display_author-1399990-40.patch, failed testing.

artusamak’s picture

Status: Needs work » Needs review
StatusFileSize
new27.84 KB
new2.04 KB

I tried to run the test suite locally but it doesn't work.

I'm totally stuck on the RDF errors generated by the test suite.
If i try to test the output of the RDF metadata manually there is only one content type that shows the schema:author and schema:dateCreated meta. It's the article content type.
No matter how i look at this, i can't figure out how the RDF module works and why it only inject the metadata for the article content type.

On top of that, i don't understand the other failing tests, it seems completely unrelated so i have no idea how to debug them.

Help will be appreciated. Thanks.

Status: Needs review » Needs work

The last submitted patch, 43: core-remove_display_author-1399990-43.patch, failed testing.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.0-beta1 was released on August 3, 2016, which means new developments and disruptive changes should now be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

artusamak’s picture

Status: Needs work » Needs review
StatusFileSize
new27.86 KB

Re-rolling against 8.3.x to see how things are going.

Status: Needs review » Needs work

The last submitted patch, 46: core-remove_display_author-1399990-44.patch, failed testing.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

artusamak’s picture

Assigned: Unassigned » artusamak
Issue tags: +DevDaysSeville

Working on this.

artusamak’s picture

Status: Needs work » Needs review
StatusFileSize
new36.03 KB

Just retesting to check if everything is green.
Left to be done:
* Taking care of all the view modes for a node type
* Handling the upgrade path
* Strenghten the displaySubmitted() / setDisplaySubmitted() methods.

Status: Needs review » Needs work

The last submitted patch, 50: core-remove_display_author-1399990-49.8.4.x.patch, failed testing.

artusamak’s picture

Status: Needs work » Needs review
StatusFileSize
new35.99 KB

Same as before, but this time everything should be green.

artusamak’s picture

Status: Needs review » Needs work

Tests are good now.
Back to moving it forward.

yoroy’s picture

StatusFileSize
new24.14 KB
new61.87 KB

Thank you for working on this. Testing on simplytest.me I see "submitted by submitted by" and "on on" in the output. And the values for the date format settings could be nicer.

artusamak’s picture

Status: Needs work » Needs review
StatusFileSize
new32.64 KB

Yes, it's fixed now. I simplified the way we have to configure the authored on / by base fields to reduce the code quantity.

This has been taken care of, i want to confirm that the tests are still green:

  • Strenghten the displaySubmitted() / setDisplaySubmitted() methods.

Next, this still has to be done but if the tests are green it should be easier.

  • Taking care of all the view modes for a node type
  • Handling the upgrade path
  • Testing the upgrade path

About the format date, this should be another issue, this patch keeps the format identical to what we have now. :)

yoroy’s picture

Yeah, I saw that those date formats are our default, so no need to focus on that here. Thanks!

Status: Needs review » Needs work

The last submitted patch, 55: core-remove_display_author-1399990-55.8.4.x.patch, failed testing.

artusamak’s picture

Status: Needs work » Needs review
StatusFileSize
new62.42 KB

Fixing the failure.

Status: Needs review » Needs work

The last submitted patch, 58: core-remove_display_author-1399990-58.8.4.x.patch, failed testing.

artusamak’s picture

Status: Needs work » Needs review
StatusFileSize
new33.76 KB

Now it should apply correctly.

artusamak’s picture

Status: Needs review » Needs work
artusamak’s picture

Status: Needs work » Needs review
StatusFileSize
new35.13 KB
new4.66 KB

Today's progress, submitting to the test bot to confirm that we are still green.
The hook_update_n() has been added to convert the existing node types to the new configuration.
All the view modes are taken care of when enabling / disabling the "display_submitted" flag, including "default" which is a special case.
I'm working on the test of the upgrade path now which is... complex.

An additional task to complete is to verify that the RDF metadata are still injected in the page when the authored on / by base fields are shown on a view mode.

We are close to have something finished!

artusamak’s picture

Issue summary: View changes
StatusFileSize
new41.94 KB
new6.43 KB

Here is the test for the update path. And the code removing the checkbox.
I'm going to test the RDF attributes now and we should be going for a first review.

artusamak’s picture

StatusFileSize
new40.89 KB
new9.16 KB

Hmm, i want to have the whole test suite run without the hack in testModuleConfig().
Let's hope it's still fine.

The last submitted patch, 63: core-remove_display_author-1399990-63.8.4.x.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 64: core-remove_display_author-1399990-64.8.4.x.patch, failed testing.

artusamak’s picture

Assigned: artusamak » Unassigned
Status: Needs work » Needs review
StatusFileSize
new40.87 KB
new6.24 KB

This should go green again.

artusamak’s picture

Last update of the patch remove the form element from the edit page. Time to catch a plane now, i will document and detail the patch soon after landing. :)

The last submitted patch, 67: core-remove_display_author-1399990-67.8.4.x.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 68: core-remove_display_author-1399990-68.8.4.x.patch, failed testing.

artusamak’s picture

Status: Needs work » Needs review
StatusFileSize
new41.61 KB
new8.47 KB

Fixing the wrongly assumed data structure + coding standards.

ifrik’s picture

StatusFileSize
new13.86 KB

Thanks, this looks good. I applied the patch and for on the Manage display tab of the content types there are now two extra fields that can be displayed and configured.
A bit more configuration would be nice: Is it possible to make the prefix texts "Submitted by" and "on" configurable instead of hard-coding it? Or is this beyond the scope of this issue?

There is however one thing that confuses me: The Display settings option is still available on the Edit page of the content type, and ticking or un-ticking the box now doesn't do anything. The only effect I can see is that when a new content type is created, the tick box determines whether the two fields are initially displayed or not.
Can we remove the tickbox and just by default display the two fields, and let users disable their display if desired?

artusamak’s picture

@ifrik you should try #71, i included the removal of the edit checkbox.

About your suggestion for the ability to manage the prefix text. Should we be able to edit it from the setting or from the template itself? It would be the only formatter that we have that is exposing an editable label. It may be easier to manage it from the field template since we may want to display it before, after, insert a line break and so on. What do you think would be the best option for that? Maybe this should be discussed in the next UX meeting?

artusamak’s picture

Here is the patch details & explaination. The patch enriches the display_submitted attribute that exists on the node type definition.

Instead of only being a flag, it's now a flag + two base fields configurations on the manage display tab of the nodes bundles.

Previously the display submitted attribute was controlled from the edit page of the content type. This field now disappears.

Screenshot content type edit before

Screenshot content type edit after

If the flag is set to TRUE, the authored on and authored by base fields are shown in the content region of each view mode.

Screenshot article manage display tab before

Screenshot article manage display tab after

If the flag is set to FALSE, the base fields are hidden.

Screenshot page manage display tab before

Screenshot page manage display tab after

Due to the fact that this configuration was for every single view mode of the bundle, this configuration still applies to every view modes. In a following issue, it will be interesting to control that independently for each view mode.

The default output of the fields matches what was displayed previous when the display submitted attribute was enabled.

Screenshot node article view

Screenshot node page view

The users can now also control the output of those two information independently through the available formatters settings.

Screenshot authored date formatter settings
Screenshot authored by formatter settings

About the tests updates. Since the node entity type now always has at least two base fields attached to the manage display tab, we can not keep the ManageDisplayTest applying to nodes. We must switch to another entity type that is able to not have any fields. The Block entity type is a good candidate for that.

Due to how the base fields definition are attached to the content types, it's impossible to attach them before saving the content type. For this reason, we need a node_set_display_submitted() function doing the job. It's similar to node_add_body_field().

The methods NodeType::displaySubmitted() and NodeType::setDisplaySubmitted() should probably be marked as deprecated to keep the base fields as independant as any others.

ifrik’s picture

StatusFileSize
new14.26 KB

Thanks Artusamak,
I've applied patch #71 now and there is no more tab on the edit page.

About the option to change the prefix wording: I suppose I was assuming functionality that comes from the Display suite module, not from core.

But... if "Submitted by" and "on" are the prefixes: What are the labels?
The "Manage display" page still has the default option to set show them (see screenshot), but nothing gets displayed.
My assumption was that if I choose Label > Above (or Inline), "Authored by" will be displayed just as the label of other fields, but it's not.

kmajzlik’s picture

function node_update_8400 already defined in 8.4.x-dev so the patch in #71 is not working with the dev version of core.

ifrik’s picture

Retesting the patch #71, because the patch doesn't seem to apply for 8.4.x anymore

Status: Needs review » Needs work

The last submitted patch, 71: core-remove_display_author-1399990-71.8.4.x.patch, failed testing. View results

lplp’s picture

Status: Needs work » Needs review
StatusFileSize
new41.63 KB

Rerolled patch in #71 after it didn't seem to apply for 8.4.x, applied fine.

Status: Needs review » Needs work

The last submitted patch, 79: 1399990-79.patch, failed testing. View results

lplp’s picture

Status: Needs work » Needs review
StatusFileSize
new654 bytes
new42.03 KB

Double declaration of node_update_8400, changed one to 8401.

ifrik’s picture

Status: Needs review » Needs work

The patch applies again, the fields can be hidden, and the prefix can be turned on an off.

But if the fields are moved it the Display Form management, they are nonetheless displayed in the same order.

yoroy’s picture

Confirming #82. It does let me hide either date or author, or both, but when both are set to be displayed they are always shown together and as the first element below the title.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

kmajzlik’s picture

Last patch #81 is incompatible with 8.4.0-alpha1

Bensbury’s picture

Hello.
Looking clarification here as I am converting from D7 to D8.
It didn't help to realise the ability to disable the Author Information was moved. Mainly because it is not showing as described in this thread and I ended up here looking for a solution.
When I look in Manage Display for the Author Fields with the intention to hide them (as shown above in the thread's images), there are no fields in the interface to hide. However they do exist in the Manage Form Display.

From what I understand at the end of this thread, whatever has been developed no longer works with the latest Drupal 8 (8.4.2)?
So I am a bit confused what we should do to manage this.
Wouldn't this be tested before adding to the stable version of Drupal 8?
Am I doing something wrong and there is a UI way to hide the fields?

Or has this functionality not been implemented fully yet and until it is the only way to hide the author information would be some kind of preprocess function or wiping it out from a theme template?

Thanks,
Ben.

artusamak’s picture

Hello Bensbury,

This issue is still not completed so don't worry, there will be more tests before it's added into Drupal core (whatever the version is).
I re-rolled the patch to work more on it and fix the feedbacks posted in the previous comments.
At the moment, the solution is still to use the checkbox of the content type edit screen to hide the author and creation date of your node.
It will change once this patch is working but there will be an upgrade path.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

k4v’s picture

Assigned: Unassigned » k4v
k4v’s picture

StatusFileSize
new41.04 KB

Reroll for 8.6.x.

As core/modules/simpletest/src/ContentTypeCreationTrait.php is deprecated, I transfered the changes to /core/modules/simpletest/src/ContentTypeCreationTrait.php

k4v’s picture

Status: Needs work » Needs review
k4v’s picture

StatusFileSize
new42.24 KB
new1.2 KB

Add missing Test from old patch.

Status: Needs review » Needs work

The last submitted patch, 92: 1399990-91.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

artusamak’s picture

Assigned: k4v » artusamak
Issue tags: +DevDaysLisboa

It's once more Drupal Dev Days season. Let's try this again!

berdir’s picture

artusamak’s picture

Hmm, indeed. I will test the patch from there to see if there is still a delta to address. It's definitely related. Thanks.

artusamak’s picture

Assigned: artusamak » Unassigned
StatusFileSize
new36.19 KB

It looks related, we should wait for it to be committed to update how the rendering of the fields is going to be managed.
Meanwhile, here is a reroll for 8.6.x and a slight update to leverage as suggested the label configuration (inline / above) to show / hide the "submitted by" value. It's less pertinent for the authored on field so i only applied that to the authored by field.

artusamak’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 97: core-remove_display_author-1399990-97.8.6.x.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

berdir’s picture

+++ b/core/lib/Drupal/Core/Field/Plugin/Field/FieldFormatter/TimestampFormatter.php
@@ -183,6 +197,9 @@ public function viewElements(FieldItemListInterface $items, $langcode) {
         '#markup' => $this->dateFormatter->format($item->value, $date_format, $custom_date_format, $timezone, $langcode),
       ];
+      if ($this->getSetting('show_prefix_label')) {
+        $elements[$delta]['#prefix'] = $this->t('on ');
+      }
     }

I'm not sure if that makes sense with multilingual and e.g. russian?

Instead it should be maybe part of the date format? but it's still not a full sentence that can be translated in any meaningfull way.

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

ilya.no’s picture

Attaching re-rolled patch and interdiff. Working fine. There were some problems with interdiff generating, so there is no updates for testNodeCreation() function, although they are presented.

andypost’s picture

  1. +++ b/core/lib/Drupal/Core/Field/Plugin/Field/FieldFormatter/TimestampFormatter.php
    @@ -183,6 +197,9 @@ public function viewElements(FieldItemListInterface $items, $langcode) {
    +        $elements[$delta]['#prefix'] = $this->t('on ');
    

    Is there a reason to have a space after "on"?

  2. +++ b/core/modules/field_ui/tests/src/Functional/ManageDisplayTest.php
    @@ -173,13 +173,14 @@ public function testSingleViewMode() {
         // Create a fresh content type without any fields.
    -    NodeType::create([
    ...
    +    BlockContentType::create([
    

    why it changing a test to use block instead of node, also comment is wrong

  3. +++ b/core/modules/node/content_types.js
    @@ -36,17 +36,6 @@
    -      $context.find('#edit-display').drupalSetSummary(function (context) {
    

    source file js.es6 should be fixed too (before this one)

  4. +++ b/core/modules/node/node.install
    @@ -281,3 +281,13 @@ function node_update_8700() {
    +function node_update_8701() {
    +  $node_types = \Drupal\node\Entity\NodeType::loadMultiple();
    +  foreach ($node_types as $node_type) {
    +    node_set_display_submitted($node_type);
    

    should be post update hook

  5. +++ b/core/modules/node/src/NodeTypeInterface.php
    @@ -45,6 +45,8 @@ public function setNewRevision($new_revision);
    +   * ¶
    
    @@ -53,6 +55,8 @@ public function displaySubmitted();
    +   * ¶
    

    wrong changes, needs to remove extra trailing whitespace

ilya.no’s picture

StatusFileSize
new36.57 KB
new17.64 KB

Attaching updated patch.
Concerning second point, there is explanation in comment #74.
Concerning first point, we need space after 'on', because it's prefix and without this space if we enable this option, date and 'on' will be shown together without space, which is confusing. Reading comments of this issue I'm not sure I get the reason, why do we use prefix at all. I think, if we move these 2 fields to the display, we should provide labels for them and thus we don't need any prefix. But currently even if we set label to be shown, it's not shown. So now we can only show date itself or date with 'on' before it. But it would make more sense to use configurable label, like 'Authored on', so we can change its text and remove 'on' part and thus no need for prefix. But this needs more changes, I guess.

davidferlay’s picture

Issue summary: View changes
StatusFileSize
new7.11 KB
new30.61 KB
new13.57 KB
new2.24 KB

Hi all)

  • I tested #108 patch and it works great :

Looking good thanks to Interface translation :

In Display settings :

@ilya.no

We should provide labels for them and thus we don't need any prefix. it would make more sense to use configurable label, like 'Authored on', so we can change its text and remove 'on' part and thus no need for prefix.

I agree with that. Also "on" should be changed for "Authored on: " (including space) to make it similar to current "Authored by: "

Currently, "on" string can be updated by user in "Interface translation" UI anyways, so current result is quite good already imho

andersen_ti’s picture

StatusFileSize
new37.17 KB
new4.32 KB

cutomizable label added.

piggito’s picture

+++ b/core/lib/Drupal/Core/Field/Plugin/Field/FieldFormatter/TimestampFormatter.php
@@ -82,7 +82,7 @@
-      $container->get('entity_type.manager')->getStorage('date_format')
+      $container->get('entity.manager')->getStorage('date_format')

Don't use entity.manager this service is deprecated so need to revert this line.

+++ b/core/lib/Drupal/Core/Field/Plugin/Field/FieldFormatter/TimestampFormatter.php
@@ -91,11 +91,12 @@
-      'date_format' => 'medium',
-      'custom_date_format' => '',
-      'timezone' => '',
-      'show_prefix_label' => FALSE,
-    ] + parent::defaultSettings();
+        'date_format' => 'medium',
+        'custom_date_format' => '',
+        'timezone' => '',
+        'show_prefix_label' => FALSE,
+        'prefix_label' => 'Authored on: '
+      ] + parent::defaultSettings();

We don't need extra spaces on this identation so this should be reverted as well.

In addition to these changes we need reroll of patch for 8.8

andypost’s picture

Status: Needs work » Needs review
StatusFileSize
new36.81 KB

Here's a reroll of 108 for 8.8.x, while rerolled I found that many tests affected - sounds like some trait could be useful

Patch in #110 looks interesting but needs more work - maybe separate issue

  1. +++ b/core/lib/Drupal/Core/Field/Plugin/Field/FieldFormatter/TimestampFormatter.php
    @@ -91,11 +91,12 @@
    +        'prefix_label' => 'Authored on: '
    
    @@ -198,10 +205,10 @@
    +        $elements[$delta]['#prefix'] = $this->t($this->getSetting('prefix_label'));
    

    Additionally to #111 - new settings needs config schema ("label" type) to allow to translate this option with config

  2. +++ b/core/modules/rdf/tests/src/Functional/StandardProfileTest.php
    @@ -98,14 +98,14 @@
    -  public static $modules  = ['field_ui'];
    +  protected $articleCommentUri;
    ...
    -  protected $articleCommentUri;
    +  public static $modules  = ['field_ui'];
    

    why that changed? scope creep

Status: Needs review » Needs work

The last submitted patch, 112: 1399990-108-reroll.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

sorlov’s picture

Updated rerolled patch from #110

sorlov’s picture

Updated rerolled patch from #112 with changes from #110 + fixed notices from above

andypost’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 115: core-remove_display_author-1399990-115.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

andypost’s picture

  1. +++ b/core/config/schema/core.entity.schema.yml
    @@ -340,6 +340,9 @@ field.formatter.settings.timestamp:
    +    prefix_label:
    +      type: string
    

    is "label" top be translatable

  2. +++ b/core/config/schema/core.entity.schema.yml
    @@ -340,6 +340,9 @@ field.formatter.settings.timestamp:
     field.formatter.settings.author:
    

    probably it should affect this one too

  3. +++ b/core/lib/Drupal/Core/Field/Plugin/Field/FieldFormatter/TimestampFormatter.php
    @@ -114,11 +115,22 @@ public function settingsForm(array $form, FormStateInterface $form_state) {
    -      '#title' => $this->t('Show the "On" prefix'),
    +      '#title' => $this->t('Show the prefix'),
    

    "the" is not needed here

  4. +++ b/core/lib/Drupal/Core/Field/Plugin/Field/FieldFormatter/TimestampFormatter.php
    @@ -198,7 +211,8 @@ public function viewElements(FieldItemListInterface $items, $langcode) {
    +        $elements[$delta]['#prefix'] = $this->t($this->getSetting('prefix_label'));
    

    this is wrong, translation should work on config level

adamps’s picture

I am the main developer of the similar issues #2923701: Mechanism to disable preprocessing of node base fields so they can be configured via the field UI/#3036862: Expose Title and other base fields in Manage Display in Drupal Core and #2353867: [META] Expose Title and other base fields in Manage Display and I've just found this issue - thanks @dww for adding a 'related issue' entry that guided me here.

Back-compatibility

In those issues above we found that we had to take elaborate precautions to ensure back-compatibility for all existing sites. My understanding is that any patch must not affect the rendered output of any site, whatever choices the developer of the site made in terms of display settings, template overrides, hook implementations and so on. This view was confirmed by the review comments that we got from key Drupal experts and core committers. My first impression is the patch on this issue breaks back-compatibility, for example:

  1. A site may have overriden the node template to say "Another cool article by XXX". After this patch it would potentially be "Another cool article by Submitted by XXX".
  2. A site may already call setDisplayConfigurable('view', TRUE) but rely on the default view settings. This patch changes the default view settings so will change the display on that site.

The approach that I am proposing in the other issues takes more time, but is BC. There would be several commits spread across 2 major Drupal versions, maybe something like this:

  1. NOW Introduce an option to use manage display, default to off
  2. LATER Create a warning if the option isn't set
  3. NEXT MAJOR VERSION Remove the option and always use manage display

Other comments

  1. The change in template_preprocess_node() is not the correct approach regarding the BC mechanism. When using manage display, the flag enable_base_field_custom_preprocess_skipping must be set. Once you have done that, you can leave node.html.twig unchanged - the code within if display_submitted will not be used, and the fields will be rendered in their correct place according to the manage display settings.
  2. Surely the username formatter is generic and can be used on any field and any entity type?? It doesn't seem right to try to alter it for this one specific purpose. Change of the username template to add extra text "Submitted by" - doesn't this mean that anywhere on any site where there the username template is used those words will appear? A lecture content type with a user reference field "Lecturer" previously displayed "Lecturer: Einstein" and will now say "Lecturer: Submitted by Einstein".
  3. Same problem with TimestampFormatter - every timestamp on the site would by default have "Authored on" added to it.
  4. Currently the node template exposes a single translation of Submitted by {{ author_name }} on {{ date }}. This allows the translator to reorder any of those 4 pieces relative to each other. It looks like the current patch would assume that submitted by comes exactly before author_name, on exactly before date etc and this likely won't be true in some languages. Also the manage display settings created by an English speaker would put author followed by date but the sentence correct order might be the other way round in a different language which is a problem a multi-lingual site because the same manage display settings are used for all languages. So I propose that there is a requirement to continue to expose a single sentence for translation.
  5. This patch will likely expose #2993647: Correctly determine when to display fields as inline so probably is blocked on that.

Strategy

It doesn't seem sensible to have 2 different initiatives/issue to solve the same problem. We should figure out how to combine issues and work together. Note that this issue is just one special case, and there are various other fields on node and other entity types - in particular the comment entity has almost exactly the same "submitted by". The other set of issues #2353867: [META] Expose Title and other base fields in Manage Display appear to have more breadth of scope and an overall big picture / sequence of events. They also have ideas to solve some of the "other comments" I raised above.

ilya.no’s picture

Attaching patch with fixes for comment #118. About #119, agree, that we need to consolidate our effort and improve BC aspect.

adamps’s picture

So in terms of general approach...

1) My understanding of Drupal back-compatibility is: we want to do something that's not BC so we need to introduce a new option then deprecate the old way. People can move their sites over gradually and there is plenty of testing along the way.

Hence there needs to be a GUI config option called something like "Show extra fields in 'Manage Display' settings". If this is enabled then we get the new behavior. Thoughts anyone???

2) There are lots of other fields apart from these two that the same applies to. If we are having a GUI option then it would be confusing and complicated to have lots of separate options, so it seems that we should have just one option for all of Core. Make sense???

If you are in agreement then what we need to do is #3036862: Expose Title and other base fields in Manage Display in Drupal Core which will depend on #3033301: Add formatters and other mechanisms as alternative to base fields directly in entity templates. So the scope is bigger and more challenging for sure, but also it would have a much bigger impact. The work on this issue will give us a start for sure although it will need tweaking a little.

What does everyone here say?

murz’s picture

Together with "Created on" field will be good to see also "Updated on" field, can we add this in current issue, or better to submit separate issue?

adamps’s picture

@Murz I think there are two separate cases:

This issue/#3036862: Expose Title and other base fields in Manage Display in Drupal Core cover fields that are already displayed in Core but are missing from Manage Display. There is widespread enthusiasm for adding them to manage display.

The "Updated on" information is different as the information is not currently displayed in Core or available on templates. I would guess this was a deliberate choice by the Core developers - there are maybe 4-5 node base fields not configurable by default as most people won't want to display them so it would be too much clutter/confusion. As I explained to you on the other issue where you raised this, it's very easy for a module/hook to expose the extra field. I believe Display Suite for example exposes almost everything possible and sometimes gets criticised for that. However if I haven't convinced you then of course feel free to raise a separate issue.

YurkinPark’s picture

YurkinPark’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 124: core-remove_display_author-1399990-124.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

YurkinPark’s picture

Status: Needs work » Needs review
StatusFileSize
new45.29 KB
new7.44 KB

Status: Needs review » Needs work

The last submitted patch, 127: core-remove_display_author-1399990-127.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

YurkinPark’s picture

Status: Needs work » Needs review
StatusFileSize
new47.27 KB
new1.63 KB

Status: Needs review » Needs work

The last submitted patch, 129: core-remove_display_author-1399990-129.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

adamps’s picture

Status: Needs work » Closed (duplicate)

In #119 and #121 I explained in great detail the problems with the approach in this issue. I asked for any comments and nobody disagreed.

I don't think it's helping to continue to post patches here with exactly the same problems and no explanation. Let's close this one as a duplicate of #2353867: [META] Expose Title and other base fields in Manage Display.

If anyone disagrees of course reopen it - but please explain why you think the approach here can work in light of my comments.