Problem/Motivation
On small screen, the table on the Extend page is collapsed and the module descriptions are not visible. In order to display them the user has to click on "Show all columns". This label is not very useful because users might not even know that they are looking at a table, and not just a list with tick boxes.
To hide the description, the label is even more confusing "Hide lower priority columns"
Proposed resolution
Replace the extend tab module description labels with "Show description" and "Hide descriptions"
this.showText = Drupal.t('Show all columns');
this.hideText = Drupal.t('Hide lower priority columns');
Remaining tasks
User interface changes
Screenshots
Before
After
Steps to check
- Go to extend page (/admin/modules)
- Set screen size <=959px
Testing Steps
- Download #29 Patch locally and apply it.
- Clear cache.
- Navigate to structure -> view and edit any view which contains table format.
- Check in mobile view version, Lables are reflected as per Proposed resolution.
API changes
Data model changes
Comment | File | Size | Author |
---|---|---|---|
#33 | after-patch-2.jpg | 711.94 KB | ranjith_kumar_k_u |
#33 | after-patch-1.jpg | 134.43 KB | ranjith_kumar_k_u |
#33 | before-patch-2.jpg | 195.68 KB | ranjith_kumar_k_u |
#33 | before-patch-1.jpg | 132.29 KB | ranjith_kumar_k_u |
#32 | 2575749-After-Apply-Patch-Hide-description.png | 508.21 KB | Rassoni |
Issue fork drupal-2575749
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #2
joyceg CreditAttribution: joyceg commentedComment #3
joyceg CreditAttribution: joyceg commentedComment #4
joyceg CreditAttribution: joyceg commentedComment #6
jcnventura CreditAttribution: jcnventura at Wunder commented@joyceg First of, the patch fails to apply because it's including temporary files. Also, you do realize that this function is applicable to all of Drupal? This is a generic responsive table function which may not even be associated with any description.
If you can fix it, keeping it generic, and solving only the extend page text, great! If not, this might be a candidate for closing with "Works as designed".
Comment #7
joyceg CreditAttribution: joyceg commentedComment #8
joyceg CreditAttribution: joyceg commentedComment #10
joyceg CreditAttribution: joyceg commentedComment #11
GoZ CreditAttribution: GoZ at Centarro commentedI agree with @jcnventura :
Label for
this.showText
andthis.hideText
are displayed and show / hide columns depending of class defined byRESPONSIVE_PRIORITY_LOW
orRESPONSIVE_PRIORITY_MEDIUM
.For example, in
\Drupal\comment\Form\CommentAdminOverview
,posted_in
andchanged
values are displayed asRESPONSIVE_PRIORITY_LOW
, so not descriptions at all.Unless you find better label description, this looks like Work as designed.
Comment #17
ifrikComment #25
Nikhil_110 CreditAttribution: Nikhil_110 at Srijan | A Material+ Company commentedAttached patch against Drupal 9.x, 10.1.x
Comment #26
_pratik_ CreditAttribution: _pratik_ as a volunteer and at Specbee for Drupal India Association commentedRerolled patch as per #25.
Thanks
Comment #27
_pratik_ CreditAttribution: _pratik_ as a volunteer and at Specbee for Drupal India Association commentedComment #28
needs-review-queue-bot CreditAttribution: needs-review-queue-bot as a volunteer commentedThe Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".
Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.
Comment #29
ranjith_kumar_k_u CreditAttribution: ranjith_kumar_k_u as a volunteer and at Srijan | A Material+ Company for Drupal India Association commentedThis is a try as per comments #6 and #11
Comment #30
ranjith_kumar_k_u CreditAttribution: ranjith_kumar_k_u as a volunteer and at Srijan | A Material+ Company for Drupal India Association commentedComment #31
quietone CreditAttribution: quietone at PreviousNext commentedThis issue is changing the user interface, so before and after screenshots are needed. Put them in the Issue summary or at least a link so that reviewers can see the latest work. See the accessibility core gate.
And this will need a usability review, adding tag.
The Issue Summary needs to be updated. The proposed resolution is to replace labels but does not say which labels are being replaced. And the remaining tasks should state what needs to be done.
I have not reviewed the patch.
Comment #32
RassoniTested locally after applying #29 patch changes are reflected as per proposed resolution. The table has
module-list
class according to labels are replaced with Proposed resolution. Attached screenshot and video for reference.Steps to Test:
Comment #33
ranjith_kumar_k_u CreditAttribution: ranjith_kumar_k_u as a volunteer and at Srijan | A Material+ Company for Drupal India Association commentedComment #35
smustgrave CreditAttribution: smustgrave at Mobomo commentedIssue summary update has not happened.
@rckstr_rohan removing credit for the empty commit in #34.
Comment #36
sahil.goyal CreditAttribution: sahil.goyal as a volunteer and at Srijan | A Material+ Company for Drupal India Association commentedUpdating the issue summary
Comment #37
Nikhil_110 CreditAttribution: Nikhil_110 at Srijan | A Material+ Company commentedComment #38
Nikhil_110 CreditAttribution: Nikhil_110 at Srijan | A Material+ Company commented