When user login, "Member for" label is invisible

Possible solution:

CommentFileSizeAuthor
#61 2638250-61.patch6.58 KBkksandr
#58 interdiff-2638250-50-57.txt5.58 KBmohit_aghera
#58 2638250-57.patch9.59 KBmohit_aghera
#54 2638250-54.patch765 bytesMedha Kumari
#53 2638250-53.patch765 bytesMedha Kumari
#52 2638250-52.patch898 bytesMedha Kumari
#51 Before-patch.png34.39 KBMedha Kumari
#51 After-patch.png44.43 KBMedha Kumari
#51 2638250-51.patch896 bytesMedha Kumari
#50 2638250-50.patch5.03 KBmohit_aghera
#35 member.for-2638250-35.patch2.74 KBgeerlingguy
#24 interdiff-2638250-22-24.txt1.03 KBAdamPS
#24 member.for-2638250-24.patch2.78 KBAdamPS
#22 member.for-2638250-22.patch2.84 KBAdamPS
#20 member.for-2638250-20.patch2.88 KBAdamPS
#16 interdiff-2638250-15-16.txt600 bytesmarkhalliwell
#16 2638250-16.patch1.16 KBmarkhalliwell
#15 the_label_member_for-2638250-15.patch695 bytesAdamPS
#6 the_label_member_for-2638250-6.patch1.05 KBmarkhalliwell
Screen Shot 2015-12-20 at 11.53.11.png93.18 KBVladimirAus
Screen Shot 2015-12-20 at 11.53.30.png94.43 KBVladimirAus
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

VladimirAus created an issue. See original summary.

VladimirAus’s picture

Issue summary: View changes
markhalliwell’s picture

I don't think it should be the Bootstrap "label" (class) component at all. It should probably be an actual <label> DOM element? Or possibly just <strong>?

markhalliwell’s picture

Title: Label "Member for" is white on white background » The label "Member for" on user profiles is hardcoded markup
Project: Bootstrap » Drupal core
Version: 8.x-3.0-beta2 » 8.0.x-dev
Component: Code » user.module

Actually, this is a core issue. The problem is that user_user_view() is hard coding this "label" class instead of using a proper render array that allows themes like us to tap into it.

Core does have a "label" render element type, it uses the form_element_label theme hook. I'm not entirely sure if that would be appropriate, but it may be considering that it's already using the "item" element type (which is why it's being wrapped by form_element).

The class "label" is used by bootstrap to denote a label component, which is not what we want here. Even if we did, the point is that we couldn't modify it to add the secondary (necessary) label-default class to make it show up (as in the screenshots above).

markhalliwell’s picture

At second glance (and considering that it's already using the "item" element type), I think just moving it to the #title property should fix the problem. Let me see if I can't roll a patch quickly.

markhalliwell’s picture

Status: Active » Needs review
FileSize
1.05 KB
heykarthikwithu’s picture

@markcarver

When user login, "Member for" label is invisible

On user login, i am able to see the "Member for" label (label is not hidden)

Are their any other ways to reproduce the bug?

markhalliwell’s picture

@heykarthikwithu, use the Bootstrap base theme. However, that isn't what this issue is about. The label not showing up (because it's white text on white background) is just a side affect of this markup being hardcoded.

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

hkirsman’s picture

On my site the fields are in <div class="field__label">Label here</div> so after applying the patch there's no formatting.

Also what bugs me is that there's no option in the admin to set the label to above, inline or hidden.

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

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should 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.

mlncn’s picture

Agree with needing to make the "Member for" output easily overridable with a template, but this changes markup from what is currently output— and doesn't actually provide any template suggestions to make overriding easy.

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

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should 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.

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

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should 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.

AdamPS’s picture

Title: The label "Member for" on user profiles is hardcoded markup » The label "Member for" on user profiles is hardcoded markup that is different from other user fields
Version: 8.4.x-dev » 8.5.x-dev
FileSize
695 bytes

Re-roll the last patch. There are still outstanding questions, but it definitely helps solve the problem "'Member for' is displayed differently from other user fields".

@mlncn

this changes markup from what is currently output

True , but I would say that's because the current output is wrong. Without this patch "Member for" is quite different from any other fields that you add using /admin/config/people/accounts/fields - it is h4 and no other field is. After this patch, they are almost identical - the only thing missing is a colon after "Member for".

doesn't actually provide any template suggestions to make overriding easy

Good point.

@hkirsman

Also what bugs me is that there's no option in the admin to set the label to above, inline or hidden.

That's a limitation of hook_entity_extra_field_info which is what is being used here.

Arguably a better solution would be to expose the 'created' base field directly, which is closer to what happens on a node. However I'm not sure how feasible it is to change this now - could be tricky to avoid back-compatibility problems for existing sites??

@markcarver

The class "label" is used by bootstrap to denote a label component, which is not what we want here.

The class "label" seems to be used in several places by Drupal core. I agree it's unfortunate that the bootstrap framework uses the same class for a different reason and I have raised #2923978: Drupal labels disappear for that. However that seems to be a problem for one specific contrib theme, and it shouldn't stop us from using the label class if that is the right thing for Drupal core.

markhalliwell’s picture

This adds a theme hook suggestion so it can be targeted.

---

That being said, this "field" should really be converted into an actual field, not some arbitrary afterthought injected in an alter hook.

AdamPS’s picture

Status: Needs review » Reviewed & tested by the community

Thanks @markcarver, that seems like the right fix here - it addresses the specific problems in the title, and is definitely a step in the right direction.

If anyone feels inclined they could raise a follow-on issue for "use an actual field".

xjm’s picture

Status: Reviewed & tested by the community » Needs review
Issue tags: +Needs accessibility review

Hm, not sure about this change. It does change the markup and the appearance, including the layout of the element so that it's now on two lines instead of a single line. It's also switching the field from a semantic element in HEAD (<h4>) to <label>, which I don't think is supposed to be used for non-form elements.

Tagging for accessibility review to double-check that.

AdamPS’s picture

@xjm thanks for your review.

Do you think it would be better to fix this 'properly'? Now that we have #2392845: Add a trait to standardize handling of computed item lists maybe we could swap 'member_for' from hook_entity_extra_field_info into baseFieldDefinitions with setComputed(TRUE)? Then it definitely would be formatted exactly like all other fields. The core installation profiles could set 'label' => 'inline' to make it on one line. It would be a more complex fix, not sure if anyone here would have time to do it.

AdamPS’s picture

Here's an alternative approach switching to a computed field. It seems much cleaner and the output is identical to other fields of course. It also allows the site builder to customise the format.

The patch likely needs more work to fix tests and provide a hook_update. However I'd like to get some feedback on the general concept first.

Warning: with this patch you will see this issue #2686409: Time Ago summary does not render on Manage Display for Timestamp and Datetime fields

Status: Needs review » Needs work

The last submitted patch, 20: member.for-2638250-20.patch, failed testing. View results

AdamPS’s picture

markhalliwell’s picture

It's also switching the field from a semantic element in HEAD (<h4>) to <label>, which I don't think is supposed to be used for non-form elements.

If this is the case, then I would imagine that all item render elements (\Drupal\Core\Render\Element\Item) should be using a different #theme_wrapper than form_element. Regardless, that would be out of scope for this issue and is really an entirely separate issue from this one.

That being said, I really do prefer the computed field approach since it doesn't deal with a custom render array at all and allows an actual field to be rendered/targeted. Plus converting this into a "proper" field means that it doesn't use <label> and users can choose where said label is displayed, so "win-win-win".

---

  1. +++ b/core/modules/user/src/Entity/User.php
    @@ -510,6 +510,23 @@ public static function baseFieldDefinitions(EntityTypeInterface $entity_type) {
    +        'settings' => [
    +          'future_format' => '@interval hence',
    +          'past_format' => '@interval',
    +          'granularity' => 2.
    +        ],
    

    The only setting that needs to be here is past_format. The rest can be removed since they are the same as \Drupal\Core\Field\Plugin\Field\FieldFormatter\TimestampAgoFormatter::defaultSettings.

  2. +++ b/core/modules/user/src/Entity/User.php
    @@ -510,6 +510,23 @@ public static function baseFieldDefinitions(EntityTypeInterface $entity_type) {
    +        'weight' => 0,
    
    +++ b/core/modules/user/user.module
    @@ -175,12 +175,6 @@ function user_entity_extra_field_info() {
    -    'weight' => 5,
    

    I think the weight should be the same as the original: 5.

AdamPS’s picture

Status: Needs work » Needs review
FileSize
2.78 KB
1.03 KB

Comments done thanks.

I would be very grateful if someone can explain why my patches for the new file are failing to apply. I ran git diff as normal. The error output is
error: dev/null: No such file or directory

Status: Needs review » Needs work

The last submitted patch, 24: member.for-2638250-24.patch, failed testing. View results

AdamPS’s picture

I would be very grateful if someone can explain why my patches for the new file are failing to apply.

Never mind it seems to have applied this time.

idebr’s picture

markhalliwell’s picture

Issue tags: +Needs tests

The failing tests in #24 is because this new (proper) member_for field needs to be added to the expected output.

We should also probably add a test to ensure that this computed field outputs as expected? At the very least so we don't delete it or change it to a manual render array again. Idk.

AdamPS’s picture

@markcarver Thanks for the analysis. I am not familiar with JSON, so please bear with me asking a basic question. Would live sites also see the extra field appearing in JSON output? Is this definitely desirable? - or is it potentially a back-compatibility problem?

markhalliwell’s picture

Would live sites also see the extra field appearing in JSON output? Is this definitely desirable?

Yes and probably, yes.

Is it potentially a back-compatibility problem?

No, because this is adding a new field. It would only be a BC problem if existing sites relied on a field that was suddenly taken away.

AdamPS’s picture

Thanks for explaining. I'm out of time for now, so if anyone else wants to fix the tests please go ahead.

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.

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.

geerlingguy’s picture

Just ran into this for the nth time, and noting that the whole feature seems a bit strange to me; besides a certain subset of site types, the value of having "member for" as a displayed-by-default field on the user profile doesn't seem desirable. I've used CSS to hide it way too many times... but maybe for Drupal 9 we can just drop it, and if someone wants to replicate the functionality, they can either add it via theming, or by a computed field or something?

geerlingguy’s picture

Status: Needs work » Needs review
FileSize
2.74 KB

Attaching re-roll against 8.7.x (no diff, patch from #24 re-applied cleanly) for the testbot to take another stab.

And I think we would definitely need to add a test for this field (to prevent regressions), and either:

  • Add it to the default fields displayed in the user profile (to match existing functionality)
  • Not add it to the default fields (IMO cleaner... but it would be even more a breaking change)

In either case, this would be a frontend change which could cause site owners to get a little upset (their theming of the user profile would most likely be affected, especially if they hid the 'Member for' output before, and it popped back in as a new field), so it would need a change record and be in a new minor release (at least).

Status: Needs review » Needs work

The last submitted patch, 35: member.for-2638250-35.patch, failed testing. View results

geerlingguy’s picture

When I visit the 'Manage display' page for user accounts, I get the following errors with the above patch:

Notice: Undefined index: type in Drupal\field_ui\Form\EntityDisplayFormBase->buildFieldRow() (line 336 of core/modules/field_ui/src/Form/EntityDisplayFormBase.php).
Drupal\field_ui\Form\EntityDisplayFormBase->buildFieldRow(Object, Array, Object) (Line: 38)
Drupal\field_ui\Form\EntityViewDisplayEditForm->buildFieldRow(Object, Array, Object) (Line: 187)
Drupal\field_ui\Form\EntityDisplayFormBase->form(Array, Object) (Line: 117)
Drupal\Core\Entity\EntityForm->buildForm(Array, Object)
call_user_func_array(Array, Array) (Line: 518)
Drupal\Core\Form\FormBuilder->retrieveForm('entity_view_display_edit_form', Object) (Line: 275)
Drupal\Core\Form\FormBuilder->buildForm('entity_view_display_edit_form', Object) (Line: 93)
Drupal\Core\Controller\FormController->getContentResult(Object, Object)
call_user_func_array(Array, Array) (Line: 123)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 582)
Drupal\Core\Render\Renderer->executeInRenderContext(Object, Object) (Line: 124)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext(Array, Array) (Line: 97)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 151)
Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object, 1) (Line: 68)
Symfony\Component\HttpKernel\HttpKernel->handle(Object, 1, 1) (Line: 57)
Drupal\Core\StackMiddleware\Session->handle(Object, 1, 1) (Line: 47)
Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object, 1, 1) (Line: 99)
Drupal\page_cache\StackMiddleware\PageCache->pass(Object, 1, 1) (Line: 78)
Drupal\page_cache\StackMiddleware\PageCache->handle(Object, 1, 1) (Line: 47)
Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object, 1, 1) (Line: 52)
Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object, 1, 1) (Line: 23)
Stack\StackedHttpKernel->handle(Object, 1, 1) (Line: 665)
Drupal\Core\DrupalKernel->handle(Object) (Line: 19)

Notice: Undefined index: label in Drupal\field_ui\Form\EntityViewDisplayEditForm->buildFieldRow() (line 50 of core/modules/field_ui/src/Form/EntityViewDisplayEditForm.php).
Drupal\field_ui\Form\EntityViewDisplayEditForm->buildFieldRow(Object, Array, Object) (Line: 187)
Drupal\field_ui\Form\EntityDisplayFormBase->form(Array, Object) (Line: 117)
Drupal\Core\Entity\EntityForm->buildForm(Array, Object)
call_user_func_array(Array, Array) (Line: 518)
Drupal\Core\Form\FormBuilder->retrieveForm('entity_view_display_edit_form', Object) (Line: 275)
Drupal\Core\Form\FormBuilder->buildForm('entity_view_display_edit_form', Object) (Line: 93)
Drupal\Core\Controller\FormController->getContentResult(Object, Object)
call_user_func_array(Array, Array) (Line: 123)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 582)
Drupal\Core\Render\Renderer->executeInRenderContext(Object, Object) (Line: 124)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext(Array, Array) (Line: 97)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 151)
Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object, 1) (Line: 68)
Symfony\Component\HttpKernel\HttpKernel->handle(Object, 1, 1) (Line: 57)
Drupal\Core\StackMiddleware\Session->handle(Object, 1, 1) (Line: 47)
Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object, 1, 1) (Line: 99)
Drupal\page_cache\StackMiddleware\PageCache->pass(Object, 1, 1) (Line: 78)
Drupal\page_cache\StackMiddleware\PageCache->handle(Object, 1, 1) (Line: 47)
Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object, 1, 1) (Line: 52)
Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object, 1, 1) (Line: 23)
Stack\StackedHttpKernel->handle(Object, 1, 1) (Line: 665)
Drupal\Core\DrupalKernel->handle(Object) (Line: 19)

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.

Christopher Riley’s picture

Has this issue ever been resolved? It seems trivial I know but this has been goobered long enough and should be a simple fix.

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

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

djg_tram’s picture

Only five years and still the same bug. :-))

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

larowlan’s picture

I agree the computed field is the best approach

Berdir’s picture

Not convinced, an extra field definition has non-trivial overhead, seems quite complicated. The _only_ thing the computed field now does is display created with a different label. Otherwise it's just duplicating the created field.

We could experiment with just rendering the created field with the timestamp_ago formatter which we've had since 2014 for the extra field.

A feature to override the label for display would be useful, because then we could possibly just drop this entirely and if you want it you could show the created field with that label (or any other). There are many use cases for that.

larowlan’s picture

That sounds much better

You should be able to use a base field override to change the label, so in theory that could go in the standard profile because this feature doesn't belong anywhere but there in my book

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

mohit_aghera’s picture

- Updating the initial patch with the approach discussed in #47 and #48
- I haven't included the interdiff as it is completely different approach compared to previous patches.
- Doing one test bot run manually to see possible failures.

Medha Kumari’s picture

Status: Needs work » Needs review
Issue tags: -Needs reroll
FileSize
896 bytes
44.43 KB
34.39 KB

Reroll the patch #16 with Drupal 9.5.x

Medha Kumari’s picture

Reroll the patch #16 with Drupal 9.5.x

Medha Kumari’s picture

Version: 9.5.x-dev » 9.2.x-dev
FileSize
765 bytes

Reroll the patch #16 with Drupal 9.2.x

Medha Kumari’s picture

Reroll the patch #16 with Drupal 9.2.x

prasanth_kp’s picture

Patch #52 failed to apply on Drupal 9.2.x-dev.

mohit_aghera’s picture

I think based on feedback on #47and #48, the implementation approach in #50seems correct.

I am hiding other patches from #51 to #54

mohit_aghera’s picture

Assigned: Unassigned » mohit_aghera
Status: Needs review » Needs work
mohit_aghera’s picture

- Fixing multiple test cases and adding a configuration files on user module's config/install folder.
- Uploading and triggering test bot to see if adding file in module's config/install folder causes more failures.

- There are still a couple of test cases related cache are failing. Debugging those.
Keeping it assigned to me

smustgrave’s picture

Version: 9.2.x-dev » 10.1.x-dev
Status: Needs review » Needs work
Issue tags: +Needs Review Queue Initiative, +Needs issue summary update

This issue is being reviewed by the kind folks in Slack, #need-reveiw-queue. We are working to keep the size of Needs Review queue [2700+ issues] to around 400 (1 month or less), following Review a patch or merge require as a guide.

Tagging for an issue summary update as the current one is outdated and there is no proposed solution that matches the patch. Should happen probably first so the agreed upon solution is seen by everyone

Testing on an Umani install for 10.1.x and the label "Member for" appears

But the feature of being able to change the label as proposed in #47 I don't see.

At this time we will need a 10.1.x patch as #58 does not apply to 10.1.x

Please do not just reroll before understanding the agreed upon solution in the IS.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

kksandr’s picture

Reroll the patch #57 with Drupal 11.x