[#MDL-10201] The hide/show eye icon on labels is incorrect when AJAX is enabled

Issue Details (XML | Word | Printable)

Key:	MDL-10201
Type:	Bug
Status:	Closed
Resolution:	Fixed
Priority:	Major
Assignee:	Nicolas Connault
Reporter:	Ann Adamcik
Votes:	5
Watchers:	5

Operations

Add/Edit UI Mockup to this issue

If you were logged in you would be able to see more operations.

Moodle

The hide/show eye icon on labels is incorrect when AJAX is enabled

Created: 21/Jun/07 03:15 AM Updated: 14/Oct/09 02:04 PM

Return to search

Component/s:

AJAX

Affects Version/s:

1.8, 1.8.1, 1.9.2

Fix Version/s:

1.8.9, 1.9.5

Environment:

Any

Participants:	Ann Adamcik, Jerome Mouneyrac, Kristian Thornley, Markus Hillig, Martin Dougiamas, Nicolas Connault, Petr Skoda, Robert Puffer and Rossiani Wijaya
Security Level:	None
QA Assignee:	Rossiani Wijaya
Resolved date:	07/May/09
Affected Branches:	MOODLE_18_STABLE, MOODLE_19_STABLE
Fixed Branches:	MOODLE_18_STABLE, MOODLE_19_STABLE

Description

« Hide

The hide/show eye icon on labels always shows as open if AJAX is enabled.

To reproduce:

Make sure AJAX is enabled.
Turn editing on.
Create a label.
Hide the label by clicking the eye icon, or by going to the update label screen and setting the visible property to hide.
Navigate back to the course page (or refresh it) - the eye is now open again.

Note that the label is actually hidden from students - it is just that the eye icon is incorrect, leading teachers to believe that the label is not hidden when it really is.

All

Comments

Change History

Version Control

Sort Order:

[ Permalink | « Hide ]

Ann Adamcik added a comment - 07/Nov/07 05:21 AM

Other resources are links, with 'class="dimmed"' if they are hidden. Labels are spans, with 'class="dimmed_text"' if they are hidden. The AJAX code only checks links when determining whether or not the eye icon should be closed. It needs to check for dimmed_text spans as well. Here's a patch for 1.8+ and 1.9 -

RCS file: /cvsroot/moodle/moodle/lib/ajax/section_classes.js,v
retrieving revision 1.32.2.2
diff -c -r1.32.2.2 section_classes.js

- - section_classes.js 27 Aug 2007 17:27:00 -0000 1.32.2.2
  - section_classes.js 6 Nov 2007 14:21:19 -0000
    ***************
  - 516,522 ****
    this.id = this.getEl().id.replace(/module-/i, '');

this.hidden = false;
! if (YAHOO.util.Dom.hasClass(this.getEl().getElementsByTagName('a')[0], 'dimmed')) { this.hidden = true; }
this.hiddenStored = null;
— 516,522 ----
this.id = this.getEl().id.replace(/module-/i, '');

this.hidden = false;
! if (YAHOO.util.Dom.hasClass(this.getEl().getElementsByTagName('a')[0], 'dimmed') || YAHOO.util.Dom.hasClass(this.getEl().getElementsByTagName('span')[0], 'dimmed_text')) { this.hidden = true; } }
this.hiddenStored = null;

[ Permalink | « Hide ]

Markus Hillig added a comment - 08/Nov/07 04:13 AM

Works perfectly in Moodle 1.8.3+. Thanks for the fix, Ann.

[ Permalink | « Hide ]

Martin Dougiamas added a comment - 18/Jul/08 02:09 PM

Nicolas can you review and merge this please?

[ Permalink | « Hide ]

Kristian Thornley added a comment - 15/Aug/08 06:38 AM

Looks like this fix works partially on Moodle 1.9.2.+

Problem now is the style changes but this is not reflected in the DOM

proposed solution:

change <<Moodle Root>>/course/lib.php and make labels have the class dimmed if they are hidden and always wrapped in a div

replace lines 1350 - 1356 with
echo "<div ";
if (!$mod->visible) { echo "class='dimmed' "; }
echo ">";
echo format_text($extra, FORMAT_HTML, $labelformatoptions);
echo "</div>";

then modify css for theme so that div's with dimmed are greyed out e.g.

a.dimmed:link,
a.dimmed:visited,
div.dimmed {
color:#AAAAAA;
}

then modify and insert following code round about line 537 of <<Moodle Root>>/lib/ajax/section_classes.js (look for this line this.linkContainer = this.getEl().getElementsByTagName('a')[0]

if (YAHOO.util.Dom.hasClass(this.getEl().getElementsByTagName('div')[0], 'dimmed')) { this.hidden = true; }
if(this.linkContainer.parentNode.className == 'commands'){
this.linkContainer = this.getEl().getElementsByTagName('div')[0];
}

[ Permalink | « Hide ]

Robert Puffer added a comment - 04/Sep/08 10:40 PM

This is still an issue in 1.92+ (20080711) and I'd definitely consider it major. Not only does it display the incorrect state of the "eye" but it disallows instructors from making a resource visible once it takes on an invisible state with the eye opened. A patch would be great.

[ Permalink | « Hide ]

Markus Hillig added a comment - 26/Sep/08 10:24 PM

For what it's worth it: for Moodle 1.9.2+ I had to adapt Ann's above patch from 07/Nov/07 since now dimmed text gets enclosed between div tags on no longer between span tags. This is what I did. For me it works so far.

[root@ecampus html]# diff -c moodle/lib/ajax/section_classes.js.dist moodle/lib/ajax/section_classes.js

- - moodle/lib/ajax/section_classes.js.dist 2008-07-19 02:17:39.000000000 +0200
  - moodle/lib/ajax/section_classes.js 2008-09-26 16:14:42.000000000 +0200
    ***************
  - 526,532 ****
    this.id = this.getEl().id.replace(/module-/i, '');

this.hidden = false;
! if (YAHOO.util.Dom.hasClass(this.getEl().getElementsByTagName('a')[0], 'dimmed')) { this.hidden = true; }
this.hiddenStored = null;
— 526,532 ----
this.id = this.getEl().id.replace(/module-/i, '');

this.hidden = false;
! if (YAHOO.util.Dom.hasClass(this.getEl().getElementsByTagName('a')[0], 'dimmed') || YAHOO.util.Dom.hasClass(this.getEl().getElementsByTagName('div')[0], 'dimmed_text')) { this.hidden = true; } }
this.hiddenStored = null;