MODX2.7: Top menu is broken when add new item

catsmeatman commented 5 years ago

Summary

New menu item not fit in top horizontal menu

Step to reproduce

Add new item in /manager/?a=system/action
Reload any manager page

Observed behavior

this is caused by #modx-topnav {max-width: 68em;}

Environment

MODX 2.7-advanced

jonleverrier commented 5 years ago

~Strange - when I view the max-width for #modx-topnav it is set in pixels (1200px which equates to 75em).~

screen shot 2018-11-30 at 11 29 41

~Could this be a problem caused by your custom manager theme perhaps?~

Ruslan-Aleev commented 5 years ago

@jonleverrier I updated the pure MODX 2.6.5 to 2.7 - the problem is in place. I use Windows 7, checked in last Chrome and Firefox.

JoshuaLuckers commented 5 years ago

Does clearing the browser cache help?

Ruslan-Aleev commented 5 years ago

@JoshuaLuckers It did not help me. The problem may be: 1) due to the fact that MODX was updated to 2.7, and not installed cleanly. 2) is Windows OS.

jonleverrier commented 5 years ago

In fact, after clearing my cache, I can see that max-width is set to 68em not 1200px! So ignore my previous comment :)

jonleverrier commented 5 years ago

I can't quite pin it down.

Looks like something happened to the generated css during this commit: https://github.com/modxcms/revolution/commit/f5840eeba313bda24feafa9cb01874b72243303b#diff-7e2c7fd904a55c9f817302cebbcaf068

Any ideas @Jako ?

alroniks commented 5 years ago

Looks like it comes from some library that was updated.

Jako commented 5 years ago

Hmm, some grunt process is running mad. It removed the :not css rules too.

muzzwood commented 5 years ago

Does anyone have a quick fix for this issue?

It might cause other issues but for now I've just changed line 2345 in index.css from max-width:68em; to max-width:100%;.

jonleverrier commented 5 years ago

I tried to fix over the weekend, but I can’t find any reference to max-width on #modx-topnav in the SASS files in the 2.x branch.

Any ideas where that property is coming from @jako ?

Ibochkarev commented 5 years ago

@jonleverrier

https://github.com/modxcms/revolution/blob/2.x/_build/templates/default/sass/_navbar.scss

jonleverrier commented 5 years ago

@ibochkarev - I looked at that file. No reference to max-width on #modx-topnav !

Ibochkarev commented 5 years ago

@jonleverrier Sorry, did not finish reading

Jako commented 5 years ago

The template uses node-neat and bourbon. If I am right it is caused by node-neat.

jonleverrier commented 5 years ago

Thanks @Jako - that makes sense now.

Before I try and fix, If I compile the CSS locally, the max-width value on #modx-topnav is set correctly to the old value of 1200px and not 68em?! Should that be the case?

Jako commented 5 years ago

Could you look if the missing :not (i.e. in https://github.com/modxcms/revolution/commit/f5840eeba313bda24feafa9cb01874b72243303b#diff-7e2c7fd904a55c9f817302cebbcaf068L4183) rules are there in your compiled css? Then we have to solve a compilation issue on integrator side.

jonleverrier commented 5 years ago

@Jako They are not missing in my generated CSS! So like you say, looks like it could be a compiling issue.

Jako commented 5 years ago

If I remember right, then I did not notice a CSS change, when I compiled the CSS on my installation after the bower removement. So it should be a compiling issue.

alroniks commented 5 years ago

@jonleverrier Which versions of nodejs and npm did you use?

jonleverrier commented 5 years ago

@Alroniks I'm using:

node v6.14.4
npm 3.10.10

I can't remember precisely, but I either had some problems with building the MODX CSS or there was a conflict with my other build tools, so had to down grade.

OptimusCrime commented 5 years ago

I have the same issue, running more updated versions of node and npm:

~/.../templates/default/css  on  2.x  $ node -v
v10.13.0
~/.../templates/default/css  on  2.x  $ npm -v
6.4.1

jonleverrier commented 5 years ago

@Alroniks @Jako

I've just updated to the latest version of node.js:

node v10.14.1
npm v6.4.1

I managed to compile the CSS without any errors (plenty of warnings!). The max-width on #modx-topnav is still set to 1200px (not 68em which is causing the issue).

However, the missing :not (https://github.com/modxcms/revolution/commit/f5840eeba313bda24feafa9cb01874b72243303b#diff-7e2c7fd904a55c9f817302cebbcaf068L4183) are still missing.

alroniks commented 5 years ago

@OptimusCrime Yes, I have the same versions and recompiling does not give me an any changes (it means that issue still there). So it's a problem.

OptimusCrime commented 5 years ago

@Alroniks you do properly clean the env before running the build?

rm -rf node_modules && npm i && grunt build

alroniks commented 5 years ago

@OptimusCrime Yes, nothing. Weird.

Jako commented 5 years ago

It is a sass compiling issue (the :not rules are away after grunt sass), but I had no time to locate this.

Jako commented 5 years ago

The :not rules are removed by libsass: https://github.com/sass/node-sass/issues/2330. So we have to wait for a fix there.

modxcms / revolution