search

moodle-an-hochschulen / moodle-local_boostnavigation

Moodle plugin which tries to overcome some fixed appearance behaviours of Boost's nav drawer in a clean way
GNU General Public License v3.0
39 stars 27 forks source link

Add settings to display icons to collapsed nodes #57

Closed adpe closed 4 years ago

adpe commented 4 years ago

Rework of https://github.com/moodleuulm/moodle-local_boostnavigation/pull/53 and will solve #52

Further I fixed some typos in one CSS line.

Greets, Adrian

abias commented 4 years ago

Hi @adpe ,

Thank you very much for your work on this patch and your patience with me merging it.

Finally, this PR / feature has landed in master in https://github.com/moodleuulm/moodle-local_boostnavigation/commit/a6a4749d710bd3a11c62f335c3a2b03709ca2544

I just had to slightly rework your work in a subsequent patch on https://github.com/moodleuulm/moodle-local_boostnavigation/commit/3d1ab662d857295417ae84e01e078d8efa3f0ed0, especially as your PR seemed to break the status quo of the presentation of collapsible parent nodes which didn't have an icon but also did not have an indent. To keep the status quo, I extended the binary setting which you introduced to a tertiary setting. I hope that your demand is now covered alongside with our demand.

Thanks again, Adrian, and keep up the good work!

Cheers, Alex

adpe commented 4 years ago

Thanks @abias for taking your time to integrate this functionality in to the plugin.

Unfortunately there's an issue, that the data-isexpandable and data-collapse of the parent node is not updated and kept the values of 0. We're using this attribute values to show an arrow up/down to indicate the user, that this is a collapsible node.

In our branch issue-52 this was working. Maybe this can help you, to figure out what you've done different?

HTH, Adrian

abias commented 4 years ago

Hi @adpe ,

I am sorry, but it think I missed one important note in my feedback to this pull review:

There was a major change in https://github.com/moodleuulm/moodle-local_boostnavigation/commit/f3868412c0e65b63d419882b66b45b10cebfa46b shortly before your PR landed which was necessary due to breaking changes upstream in Moodle 3.8.

With this change, I got rid of the abuse of the data-* attributes and implemented another mechanism to collapse the nodes.

This results in the fact which you are seeing: The data-* attributes are still there, but their values remain unchanged to whatever Moodle core sets them.

In addition to that, you will see that there are multiple new classes which are added to the nodes: localboostnavigationcollapsibleparent localboostnavigationcollapsedparent localboostnavigationcollapsiblechild localboostnavigationcollapsedchild

These classes should be self-explanatory and are used to control the collapsing of the nodes. Please use these classes from now on as basis for your custom styling.

Cheers, Alex

adpe commented 4 years ago

Thank you @abias

Worked like a charm and I used additional the aria-expanded attribute, which is set by Moodle core correctly.

Greets, Adrian