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

Icons missing and/or out of alignment on certain pages #838

Closed tonyjbutler closed 7 years ago

tonyjbutler commented 7 years ago

In Moodle 3.3.1 with Essential 3.3.0.3 (latest code from master):

assign_page_actionmenu assign_page_settings

participants_page_actionmenu

gjb2048 commented 7 years ago

Latest from GitHub as from the changes of last night?

tonyjbutler commented 7 years ago

Yes, I've just pulled those in the hope they would include the fix.

gjb2048 commented 7 years ago

A fix for a specific issue? If so, which one please Tony?

tonyjbutler commented 7 years ago

No, I just made sure I pulled the latest commits before reporting this issue, in case some other fix happened to have fixed this too.

gjb2048 commented 7 years ago

So was this an issue I did not know about?

tonyjbutler commented 7 years ago

I couldn't find another report for it.

gjb2048 commented 7 years ago

So it is another issue I was not aware of? Did you notice it before I changed to the new notifications and messaging menu system in the navbar which pulls in the 'plugin' custom menu?

gjb2048 commented 7 years ago

Or I might be wrong as could be the 'context_header_settings_menu' which was there before. Also in Boost there is an issue with missing icons.

tonyjbutler commented 7 years ago

As far as I can tell this is an issue that you were not previously aware of. I noticed it before pulling last night's commits, but wanted to double check that I was running the latest code before reporting it. So I pulled, cleared the caches, verified that it was still an issue and then reported it.

gjb2048 commented 7 years ago

Ok, so is Flat Navigation off please? And thanks for reporting.

tonyjbutler commented 7 years ago

Yes, flat navigation off.

gjb2048 commented 7 years ago

With the assignment pages the icons are missing in Boost, so a core issue for that: boost_assign

tonyjbutler commented 7 years ago

Aha, OK.

gjb2048 commented 7 years ago

So in Essential this will be true of the Administration menu. Issue looks like icon replacement as classes have 'fa-fw fa-fw' and no icon class.

tonyjbutler commented 7 years ago

Yes, I'd been trying to work out what 'fa-fw' meant, as there is no such icon.

tonyjbutler commented 7 years ago

So Essential is based on BS4 now?

gjb2048 commented 7 years ago

fa-fw is a FontAwesome helper class. BS4? Why do you say and think that?

tonyjbutler commented 7 years ago

Because it is inheriting Boost issues.

gjb2048 commented 7 years ago

Boost participants:

boost_p

gjb2048 commented 7 years ago

Boost is not just BS4 its templating and PHP etc. with the FontAwesome icon system, so nothing to do with BS4 but everything to do with the generation of the markup.

tonyjbutler commented 7 years ago

Right, OK.

tonyjbutler commented 7 years ago

So this will be fixed by a core fix.

gjb2048 commented 7 years ago

This smells of one core change.

gjb2048 commented 7 years ago

It would only be fixed by core if:

a) They know about it. b) It can be replicated in a core thing - Boost.

gjb2048 commented 7 years ago

Marked as core issue but I'll see if I can fix locally. And I'm not happy with the alignments anyway.

tonyjbutler commented 7 years ago

Thanks Gareth.

gjb2048 commented 7 years ago

Note to self:

.block_navigation .block_tree p.hasicon { text-indent: -21px; padding-left: 21px; <----- }

M3.3 and M3.2 not M3.1.

gjb2048 commented 7 years ago

Ok, stupid change to 'fa-fw' was made in https://tracker.moodle.org/browse/MDL-58808 -> https://github.com/moodle/moodle/commit/57ea73d02c0d431060c282a885e8c5852c12f5ba#diff-a4dcc10cfefa7c3fa24a187a61283fa1.

gjb2048 commented 7 years ago

Do you have a Moodle tracker account Tony?

tonyjbutler commented 7 years ago

Sure do.

gjb2048 commented 7 years ago

In that case, please would you be so kind as to raise an issue about the missing icons on the Boost theme in relation to it being a regression caused by MDL-58808 as pragmatically I think I'm perceived by Damyon as a 'pain in the ****' so he'll no longer take any notice of me.

tonyjbutler commented 7 years ago

OK, will do. ;-)

gjb2048 commented 7 years ago

Thank you :) - please do let me know the MDL and I'll vote for it.

gjb2048 commented 7 years ago

BTW, its only M3.3 and master issue. MDL-58808 only went there.

tonyjbutler commented 7 years ago

Cheers.

tonyjbutler commented 7 years ago

https://tracker.moodle.org/browse/MDL-59545

gjb2048 commented 7 years ago

Hi Tony,

I've worked out a local fix to put the icon back. Testing ATM.

G