Conflict with mod folder ? about moodle-mod_unilabel HOT 16 CLOSED

Hi, thank you for reporting this problem and the very useful additional information!
I have fixed this problem and published the new version in the Moodle plugin database.
Best regards
Andreas

from moodle-mod_unilabel.

Hi, thanks for the quick answer!
However it's still not working.

M.mod_folder.init_tree is still called and tries to initialize folder_tree0 which does not exist.
The only place I found js_init_call is in mod/folder/renderer.php. From what I understand it should only be called when there is a folder tree. Maybe it's a bug in mod_folder?

Best regards

from moodle-mod_unilabel.

Hi, after my patch I could not reproduce the problem anymore :(.
Can you provide some kind of step by step instructions to reproduce this problem?
Thank you!
Best regards
Andreas

from moodle-mod_unilabel.

Hi, sure, here are the steps :

• Add a folder activity with Display folder contents set to Inline on a course page
• Add a Unilabel activity of type Carousel
• Edit content of the Unilabel activity

No Javascript is loaded and we can see the error in the Console : Uncaught TypeError: this._el is null

• Change the folder activity and set Display folder contents to On a separate page
• Edit content of the Unilabel activity

Everything is working normally.

Thank you!

from moodle-mod_unilabel.

Hi, I am really sorry, but I still can't reproduce this problem anymore.
Do you really upgraded your unilabel plugin to version (2023042305 - Moodle 4.1, 2023062801 - Moodle 4.2)?
Do you use some non core course format or theme?
Did you clear all caches in your browser?
Best regards
Andreas

from moodle-mod_unilabel.

Hi @grabs,

we also ran in this issue. I just checked your recommendations.

• plugin version: 2023062801 on Moodle 4.2
• format_topics
• with cleared cache.

I also tried different browsers (e.g., FF or Chrome) and the error still occurs.
Is there anything more, I can help debugging?

Best regards
Thomas

from moodle-mod_unilabel.

Hi,
please have a look at my screenshots. That's the way I tested it.
I don't know what else I can test.
Maybe I missed something?
Best regards
Andreas

Screenshots

from moodle-mod_unilabel.

Hi,
I have followed your instructions exactly and I still get the error. If I find some time in the next weeks, I will try our steps against a vanilla Moodle just with mod_unilabel installed.

from moodle-mod_unilabel.

Hi Thomas, sorry for the long waiting.
I finally found the real problem. The activity picker inside the unilabel caused the error. While looking for activities it implicitly called the function "folder_cm_info_view()" which injected the troubling YUI code.
This should now be fixed.
Best regards
Andreas

from moodle-mod_unilabel.

Answering on behalf of @tholudwig: I just retested with and without your update and can confirm it works now.

However, maybe the API function has_view() would be the better fitting function to use.

Also: In some cases user might want to link to labels, folders learningmaps etc. directly shown on the course page. In this case one could think about still displaying them in the activity chooser. The URL could be generated like here:

https://github.com/mebis-lp/moodle-block_floatingbutton/blob/9bb5aab5d6a7b87cccdb2f39253128fea9d8df40/block_floatingbutton.php#L191

Thank you very much!

from moodle-mod_unilabel.

Hi, I'm not sure exactly what you mean.
The API function has_view() already is used to exclude labels.
The problem on this issue was the activity "folder" which has a view page. If an instance of folder is displayed inline it loads all its YUI crap just by checking the availability. The only way I can decide whether to show it in the picker or not was to check the url.
If you have a better way, I am open for it.
Best
Andreas

from moodle-mod_unilabel.

Hi, thanks for your fast response! I try to be more precise :)

I looked at your referenced commit and you are excluding the activities by checking if get_url() is empty. However, calling get_url() will trigger obtain_dynamic_data() which can lead to bigger resource usage. As far as I can see, modinfolib has a specific API function has_view() for a coursemodule info object that check's if there's an url without calling obtain_dynamic_data(), see https://github.com/moodle/moodle/blob/513f3b02c76e0321b3c57ca8014d3100b4da8d94/lib/modinfolib.php#L1675
Just a very small difference of course, but in addition to the thoughts about performance, there also might be plugins which implement these both API methods differently.

The second suggestion I was trying to make was not to exclude activities without view page at all, because teachers might sometimes maybe want to link these as well. For this case one still could just link to the corresponding course page/section and use an anchor to make the course page scroll to whereever the course module is on the page like block_floatingbutton does it.

from moodle-mod_unilabel.

Hi, I still don't get what you mean.
the method has_view() does not give me any clue of whether or not a folder is displayed inline. So give me an example how I can get this info.
Your second suggestions (btw. is unrelated to this issue) I don't understand too. A label for example has no url I can use in an href. I only could reference the section the label is assigned to. But this would be wrong in my opinion. I don't know the floating button plugin and I don't know what goal it has.
Best
Andreas

from moodle-mod_unilabel.

1. Did you look at the link I provided? Here is the PHPDoc:
   
   It clearly states that the function has_view() says if the course module has a view page or not. And it's basic implementation also does more or less similar things like calling empty($cm->get_url()). So as a course module developer I would have to overwrite this function If I want to declare, if my course module has a view page or not.

2. Yes, a little bit unrelated, but not completely, because with your fix you are just removing all course modules from the list of available course modules which do not have a view page. But maybe you want to keep them and instead of using get_url() to retrieve a url, you might want to generate the view link itself like block_floatingbutton does. Again, I provided the exact link where this is being done there. This way users would also be able to link course modules without a view page. The link would just open the course in the right section and scroll to the activity by using an anchor.

from moodle-mod_unilabel.

Hi, maybe take a look at

Best
Andreas

from moodle-mod_unilabel.

Hi again and thank you very much for your detailed reply.

1. I had another look and I'm very sorry for the confusion. You are totally right that you are already using the has_view function there (for some reason I did not see it for the first time(s), again sorry). I was just assuming that this function should do what the empty($activityinfo->url) check does later on, but apparently it does not: It in general declares if there is a view page, but mod_folder later on declares in obtain_dynamic_data that it will be displayed inline, so the has_view function does not return this state. Despite PHPDoc describe the behavior properly, I'm wondering if this is intended. So forget whatever I suggested, I was wrong there, sorry again :)

2. Yes, my intention was not to link the whole section, but generate a link like $CFG->wwwroot/course/view.php?id=COURSEID&expandsection=SECTIONNUM_WHERE_MODULE_IS_DISPLAYED#module-MODULE_ID. But I agree, that this might not be 100% stable in some course formats and it's discussable if this makes sense. Just wanted to show an possibility how one could handle these view-page-less coursemodules differently.

Thanks again for the fix!

from moodle-mod_unilabel.