-
Notifications
You must be signed in to change notification settings - Fork 65
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
inconsistent styling using bg-color mixin #304
Comments
Using AT 0.10.5 with FW 5.15.2 with Vanilla 5.7.0 |
Hi @chucklorenz, Thank you for raising this as an issue and apologies for the rather tardy response on my part. The move from The mixin itself predominately targets the block and component levels and could relatively swiftly be scaled up to also target articles. In fact, by 'limiting' the mixins application to these three levels (article / block / component) it should hopefully resolve the inconsistent application. I use the word limit here not in the physical sense of restricting the mixin but by removing reference that it can be applied at higher levels. Addressing your bullet list:
In summary, I would suggest making the following amendments:
I'm keen to know your thoughts on the above and look forward to your response. |
Hi @guywillis Extra classes and mixins are a bonus. Because these mixins are "extras," I believe the Vanilla author (you in this case) can make them work in any gosh-darn way he/she wants--as comprehensive or as granular. But I think as you do, that comments alongside the mixins ought to reflect its intended use. So ultimately I'm OK with any [or no] modification you make to the mixin code so long as the comments better reflect the mixin's intended use. An addition that I think might be helpful: in the comments identify the role of the mixin parameters: one determines the background color, one determines the font color. Might help the understanding of someone unpracticed in reading Less mixins (my hand is raised!!). Yes, I'd like to see a version of the mixin that is applied to an article and that cascades to its child blocks. I think it would be better still if it incorporated the page, too, and cascaded to all of its child containers. (Exclude the page header, I think that needs a mixin that targets it separately. So the only thing on the page level that gets modified is the background; but applying it to the page is meaningful because the cascade impacts all of its articles.) But I'd want this cascading mixin as an alternative to, not as a replacement for, targeted mixins. Can we have both? I'd still want a mixin that targets the container I applied it to and that does not cascade to its children. In other words, I'd like a mixin that changes an article's background and font color without affecting its blocks. We have a targeted mixin for the block. We'd need one for the article. I guess you could do one for the page if you felt the need for completeness, but there are other easy ways to assign a background color to a page. We have a targeted mixin for the page header, too. Perhaps one for a page footer (doesn't matter if it is not in Adapt repos)--again if you feel the need for completeness. And let's ignore the component because the standard, unmodified Adapt component is simply not constructed to take accept a background. (No padding, etc.) Is one helpful for menu items? Perhaps, but low priority for me. If we have any mixins that cascade, I think it will be important to examine the compile order. If Guy, here's my recommendation to you:
Hope this is helpful. And happy to continue the conversation here or in PM if desired. |
So far I have been unable to create such a global mixin, one whose styles are overwritten by usage on a child.
We could achieve increased specificity (and success) is we introduced another parameter representing the element: Thoughts? |
@guywillis any thoughts? |
Using AT 0.10.5 with FW 5.15.2
I like having built-in classes and mixins. Unfortunately, I feel the style inconsistencies are counter-intuitive enough to count as a bug. Or am I misunderstanding??
The image below uses Vanilla's built-in .bg-color-mixin:
@acme-slate: #2f4454;
@acme-rosefont: #da7b93;
.bg-color-mixin(acme-slate, acme-rosefont);
It was applied in the AT in page Classes.
Comments in theme-extras.less says it can be applied to course, contentObject, article, block, or component. No mention is made of style cascade. I think it is counter-intuitive to expect that applying a "bg-color" would expect a cascade. Applying a bg-color to a single article or block is an act of making an exception to the theme's general styles. I would expect that I am modifying that single element.
And the cascade is inconsistent:
Also the bg-color applied to the page awkwardly pokes through the margin on the menu item:
On the other hand, the header-color mixin which is intended for use with page headers is much much more consistent (since the rule is less expansive). The menu item, too, is unaffected.
The text was updated successfully, but these errors were encountered: