In the new module page we're collapsing the description and details for a module. It's a custom implementation and should be using the details element since that's exactly what it's made for.

The patch is pretty ugly, it's the quick and dirty version of what needs to be done for using details, it needs CSS cleanup, and proper use of the details form element.

Bonus, it removes a JS file since we're now using the same implementation as the rest of core for this. No idea why I haven't thought about it before.

Commit message:

Issue #1893072 by nod_, nagwani, Pete B: Use details element on module page.
Files: 
CommentFileSizeAuthor
#21 Screen Shot 2013-07-02 at 21.37.57.png48.96 KBsaki007ster
#21 Screen Shot 2013-07-02 at 21.38.17.png43.13 KBsaki007ster
#21 Screen Shot 2013-07-02 at 21.38.37.png65.41 KBsaki007ster
#21 Screen Shot 2013-07-02 at 21.38.52.png68.11 KBsaki007ster
#21 Screen Shot 2013-07-02 at 21.51.18.png156.25 KBsaki007ster
#21 Screen Shot 2013-07-02 at 21.52.13.png152.95 KBsaki007ster
#20 core-module_page-details-1893072-20.patch6.05 KBPete B
PASSED: [[SimpleTest]]: [MySQL] 57,980 pass(es).
[ View ]
#19 core-module_page-details-1893072-19.patch6.03 KBnod_
PASSED: [[SimpleTest]]: [MySQL] 56,006 pass(es).
[ View ]
#16 core-module-page-details-1893072-16.patch6.83 KBPete B
PASSED: [[SimpleTest]]: [MySQL] 56,067 pass(es).
[ View ]
#12 core-module-page-details-1893072-12.patch3.83 KBPete B
PASSED: [[SimpleTest]]: [MySQL] 55,678 pass(es).
[ View ]
#3 module_page_details.png45.08 KBnod_
core-module_page-details.patch3.45 KBnod_
PASSED: [[SimpleTest]]: [MySQL] 50,620 pass(es).
[ View ]

Comments

webchick’s picture

Issue tags:+Needs screenshots

How bad does it look?

webchick’s picture

Issue tags:+Usability

Also looping in the UX team.

nod_’s picture

StatusFileSize
new45.08 KB

Pretty bad :þ

nod_’s picture

Issue tags:-Needs screenshots
webchick’s picture

LOL that is "awesome" :P

nod_’s picture

It's like a teenager putting on makeup for the first time. It's all over the place.

Wim Leers’s picture

It's like a teenager putting on makeup for the first time. It's all over the place.

HAHAHHAHAHAHAHA :D

Bojhan’s picture

Oh, details. Anyways, following but consider it RTBC when it doesn't change anything visual (make sure you check all the wierdness around hover/selection).

nod_’s picture

Status:Needs review» Needs work

Reality check :)

droplet’s picture

Bonus, it removes a JS file since we're now using the same implementation as the rest of core for this. No idea why I haven't thought about it before.

Only one desktop browser, the chrome supports it.

nod_’s picture

we have a fallback, and a polyfill should be used by the time D8 ships #1839158: Replace collapse.js with a proper polyfill for <details>.

Pete B’s picture

StatusFileSize
new3.83 KB
PASSED: [[SimpleTest]]: [MySQL] 55,678 pass(es).
[ View ]

Patch to match the style of the original page using the new element.

Pete B’s picture

Status:Needs work» Needs review

Set needs review

nod_’s picture

Status:Needs review» Needs work

Real nice, thanks!

Styling almost there, still a #system-modules .description { cursor: pointer } that should be removed. And the system.module.js file and the declaration removed from system_library_info() for this file.

nod_’s picture

Closed #1898354: minimize module page's Description click area for this one. After the above comments are addressed this is RTBC for me.

( edit ) From the other issue nagwani should be credited for the patch too.

nod_’s picture

Issue summary:View changes

give comit message

Pete B’s picture

StatusFileSize
new6.83 KB
PASSED: [[SimpleTest]]: [MySQL] 56,067 pass(es).
[ View ]

Hi nod_,

Thanks a lot for the feedback. I have addresses the cursor pointer issue.

We actually still need system.modules.js because it is handling the search functionality, which needs to stay.

As an aside, I think we might need to start another issue to get test coverage for the module search, because patches #0 and #12 broke it and the test bot didn't notice?

I've fixed it now, and have removed the deprecated code from system.modules.js

Thanks,
Pete

nod_’s picture

Status:Needs work» Needs review

cool, i'll have a look. We can't test JS in core just yet. We rely on the fat module. So if we want to make sure it works, we need to write tests in that module.

nod_’s picture

cool, i'll have a look. We can't test JS in core just yet. We rely on the fat module. So if we want to make sure it works, we need to write tests in that module.

nod_’s picture

StatusFileSize
new6.03 KB
PASSED: [[SimpleTest]]: [MySQL] 56,006 pass(es).
[ View ]

Made a couple of changes. took out a couple of CSS rules, minor change in the JS to avoid reselecting something we already selected, added an ID to the details element (otherwise we end up with <a> links with no href and i don't want that :p).

For the future, you don't need post a patch with commented CSS code, or JS or PHP actually. We can see in the diff what's taken out.

Pete B’s picture

StatusFileSize
new6.05 KB
PASSED: [[SimpleTest]]: [MySQL] 57,980 pass(es).
[ View ]

Reroll against latest core.

Thanks,
Pete

saki007ster’s picture

Tested this patch manually. Working fine for chrome,mozilla,safari,IE9 and mobile
Attaching screenshots for the reference.

nod_’s picture

sounds like RTBC to me :)

Bojhan’s picture

Status:Needs review» Reviewed & tested by the community
catch’s picture

Status:Reviewed & tested by the community» Fixed

Committed/pushed to 8.x, thanks!

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

Anonymous’s picture

Issue summary:View changes

name typo