search

gjbarnard / moodle-format_topcoll

Collapsed Topics course format for Moodle.
http://docs.moodle.org/en/Collapsed_Topics_course_format
GNU General Public License v3.0
35 stars 60 forks source link

Remove override for action menu alignment #51

Closed Dagefoerde closed 5 years ago

Dagefoerde commented 5 years ago

Hi, another change: The edit action menu looks displaced. At least it is inconsistent with the display in core Moodle formats. This is what it looked like before

before

and after my change

after

The computed styles look like this...

computed styles

... so I removed the override.

Note: I couldn't find any information as to whether the CSS change was intentional or by accident. So please consider this a mere suggestion; if it was intentional feel free to close this. (Well, obviously. :slightly_smiling_face: Nevertheless, I felt that it was important to mention this.)

Dagefoerde commented 5 years ago

sorry, I hit "submit" too early. If you are reading only Github's e-mail notifications, please note that I made a major revision to the PR description. Thanks for your work, @gjb2048!

gjb2048 commented 5 years ago

Rejected as change made for https://tracker.moodle.org/browse/CONTRIB-4999 with all the reasons listed there. There is no code in CT that is not there for a reason!

Dagefoerde commented 5 years ago

Ahh, that makes sense, fair enough! Your changelog listed CONTRIB-4099 instead of CONTRIB-4999 so I wasn't looking at the right issue.

Thanks, closing! :)

Dagefoerde commented 5 years ago

Oh, I might add: Interestingly, the problem described in CONTRIB-4999 with the overlapping icon does not exist in theme_essential, even with my patch applied. This looks pretty good in essential. But still, haven't tested in Boost, Clean, or any other core theme, so you are probably very right not to merge. I ported this patch into our local fork of essential instead.