Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Increasing less variable "plone-left-toolbar-expanded" from registry breaks closed state of toolbar on mobile #99

Open
acsr opened this issue Mar 28, 2016 · 3 comments

Comments

@acsr
Copy link

acsr commented Mar 28, 2016

If you e.g. increase the less variable "plone-left-toolbar-expanded" in the resource registry control panel from 120px to 200px, the closed state of the Edittools never closes anymore completely on a mobile device.

Solution: respect or ignore change of the variable on mobile screens.

@davilima6
Copy link
Member

Not sure if Plone code should cover this kind of Site Admin mistake/responsibility. I agree however that we could add a warning paragraph before all the input boxes, similar to:

"Beware that changing some of the variables below might break parts of the user interface. For instance, if you increase "plone-left-toolbar-expanded" from 120px to 200px, Plone Toolbar will not close anymore completely on a mobile device."

@acsr
Copy link
Author

acsr commented Mar 28, 2016

@davilima6 Limit it to the first sentence. The second is too explicit.

It is not a "mistake". In fact you need to increase this value for some languages. Checking all consequences of a more general change is hard work. If you expose a variable, it should be safe to change it. If a variable name claims a property, it should deliver its promise.

If the mobile toolbar is not properly following an exposed base value, the related CSS / Less code is broken (not really, but looks wrong).

This Plone Code is related to the base Theme UX Experience. Beware that poor quality can ruin reputation. So if you start add specific warnings like that, you may end up with a lot of mess, because you need to remove them after a fix.

Robot tests help you rendering all UI elements for reviewing, but the best would be to figure out, what UI views depend on a variable in a programmatic way and limit the tests to the involved UX.

This is why we introduced tests and QA. Plone was always inspiring for me how we welcomed those technologies. Please do not stop improving.

QA Approach

In my opinion it could be possible to run a dependency check to drill down elements involved by a changed variable. Listing those involved explicitly in a warning would be acceptable for me as a last choice. Without the complexity is too high in any case.

But in case of the toolbar it is worth to fix it. I can try that, but need first to increase my Plone 5 Theming experience based on barceloneta. Sunburst had a lot of bad issues that looked bad in the early days. I think it is absolutely necessary to keep a first class reputation for Plone. Even with those small parts.

@davilima6
Copy link
Member

Sorry if I sounded as an innovation breaker, I just thought your suggestion could be too difficult to implement and proposed a different trade-off.

I agree automated testing would be best scenario. However afaik we currently do not even have unused css removal tasks on our build process so that kind of dependency mapping - which would have to include the on-the-fly compilation of the TTW defined LESS variables - seemed a bit too far off. If you think that's feasible though, I agree it'd be great to have that level of QA.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants