search

gjb2048 / moodle-theme_essential

The Essential Moodle Theme
https://gjb2048.github.io/moodle-theme_essential/
GNU General Public License v3.0
91 stars 120 forks source link

'Hide default page editing button' hides anything that could be in that place #804

Closed jojoob closed 7 years ago

jojoob commented 7 years ago

The pull request #576 which introduced the features 'Display editing button' and 'Hide default page editing button' includes some misbehaviour on hiding the page editing button. Some modules use the area "where the 'Turn editing on' button normally goes" [moodle_page::set_button() documentation] for their own functionality. So the mod_wiki dos for the wiki search.

The code which intends to remove the default page editing button doesn't care what's actually in that area before it removes the content: https://github.com/gjb2048/moodle-theme_essential/pull/576/commits/1dae2a17031c166c49dae4e53ca69d728a7003bd#diff-bb06945c9eaa3c6e1047bb59790e9755R670

Hide default page editing button = false: theme-essential-wiki

Hide default page editing button = true: theme-essential-wiki-hide

Further components that use the edit button area to display other things than the page edit button:

jojoob commented 7 years ago

This is related to #595

gjb2048 commented 7 years ago

Hi Johannes,

Thanks for the information. What version of Essential are you running please?

G

jojoob commented 7 years ago

Theme: Current head of master branch ('3.1.1.7 (Build: 2016061716)') Moodle: 3.1.4+ (Build: 20170119)

jojoob commented 7 years ago

I just imagined how simple all this could be if there were an uniform API that assists "editable" sites. *daydreaming* Maybe it could be something the $PAGE object (moodle_page class) provides. E.g.:

$PAGE->set_editable(true); // Will automatically display an edit on/off button
if ($PAGE->is_editing()) {
    // display edit forms...
    // maybe force turn off editing:
    // $PAGE->set_editing(false);
}

Do you think this is worth an issue at the Moodle tracker? Maybe I will write one.

gjb2048 commented 7 years ago

Hi Johannes,

Indeed. With the tracker you can try, but in my experience MHQ will ignore it if it can only be proven to the needs of a contributed plugin only.

Kind regards,

Gareth

jojoob commented 7 years ago

Maybe other plugin developers can point out that any plugin with editable pages can profit from it.

Anyway, have you any idea how to solve this issue and the related problems (have you noticed my comment on the closed issue #595)? There is also something strange in the fix for #789. I've commented the problematic point in code: https://github.com/gjb2048/moodle-theme_essential/commit/05513fcfd7720b0bb5412244bc0f7ea214027b78

The only way to solve this seems to be extend the whitelisting strategy you already implemented. I would propose to not display the navbar on/off switch as the default behaviour if the site is unknown to the whitelisting.

gjb2048 commented 7 years ago

Hi Johannes,

I've not looked at it yet in detail. When I get time I will.

Kind regards,

Gareth

gjb2048 commented 7 years ago

Hi Johannes,

Looking at https://github.com/gjb2048/moodle-theme_essential/pull/576 I see that it was you're code in the first place. Therefore what is your proposed fix to this please?

G

jojoob commented 7 years ago

I'm already investigating how to rewrite the functionality. I'll update this post with my findings and thoughts.

gjb2048 commented 7 years ago

Ok ta.

gjb2048 commented 7 years ago

Hi Johannes,

Any progress please?

Cheers,

Gareth

gjb2048 commented 7 years ago

Ok, plans for a better solution. Searched all code for '->edit_button(' - interesting results.

jojoob commented 7 years ago

Sorry, I was busy with another project. Thank you for fixing this.

This is what I had brought together already (listings are not complete). Maybe it is of interest in the future.

List of all files using moodle_page::set_button()

File Function Page type
admin/category.php block edit admin-category
admin/settings.php block edit admin-setting-section
calendar/export.php preferences calendar-export
calendar/managesubscriptions.php preferences calendar-managesubscriptions
calendar/view.php preferences calendar-view
course/renderer.php manage course-renderer
course/search.php search form course-search
course/view.php block edit course-view-[format]
grade/lib.php ? grade-lib
lib/adminlib.php ?
lib/pagelib.php ?
mod/book/lib.php ? mod-book-lib
mod/data/view.php mod-data-view
mod/forum/discuss.php search form mod-forum-discuss
mod/forum/index.php search form mod-forum-index
mod/forum/search.php search form mod-forum-search
mod/forum/subscribers.php manage subscr mod-forum-subscribers
mod/forum/view.php search form mod-forum-discuss
mod/lesson/locallib.php ? mod-lesson-locallib
mod/quiz/index.php mod-quiz-index
mod/resource/locallib.php mod-resource-locallib
mod/scorm/player.php mod-scorm-player
mod/wiki/pagelib.php mod-wiki-pagelib
my/index.php my-index
my/indexsys.php my-index
tag/index.php tag-index
tag/search.php tag-search
theme/essential/classes/output/core_renderer.php
user/profile.php user-profile
user/profilesys.php user-profile

Kinds of Edit Button Generation

HTML param name param values Set $USER->editing Used by
$OUTPUT->single_button adminedit on, off admin/category.php, admin/settings.php
$OUTPUT->single_button adminedit 1, 0 lib/adminlib.php
$OUTPUT->single_button edit on, off mod/data/view.php
$OUTPUT->single_button edit 1, 0 mod/book/lib.php, myindex
$OUTPUT->edit_button() edit on, off x course/view.php, tag/index.php
custom HTML form edit on, off mod/forum/subscribers.php
gjb2048 commented 7 years ago

Thanks for the information and hard work Johannes.