uiowa / uids

UI Design System
http://uids.brand.uiowa.edu
7 stars 1 forks source link

Investigate options for adding 'uids-' prefix through scss preprocessing #191

Open pyrello opened 4 years ago

pyrello commented 4 years ago

CSS classes: https://github.com/johno/gulp-class-prefix HTML: https://github.com/theKashey/gulp-html-prefix

bspeare commented 2 years ago

@pyrello can we close this? Is this still relevant?

pyrello commented 2 years ago

It may be relevant if we end up expanding the usage of this in future versions, but I am okay with closing for now because it doesn't seem like that is going to be the case for a while.

bspeare commented 2 years ago

Closing this for now.

pyrello commented 2 years ago

Re-opening this as we want to figure out how to do it in the 4.x branch.

pyrello commented 2 years ago

This looks like a path forward for generating prefixed classes in our dist CSS files: https://github.com/RadValentin/postcss-prefix-selector

This is a potential way forward for having our Vue components apply the correct prefix to class names inside the component logic: https://www.npmjs.com/package/classnames-prefix

pyrello commented 2 years ago

@quamsta Since this is a request you have made, it would be helpful if you could help provide some context and the expected scope of this. Is this just for cases where someone is consuming the exported CSS files? Would you expect this to work inside Vue component or in imported SASS files?

My initial feeling is that this would just be for generating namespaced CSS files for cases where it is only being implemented that way and everything beyond that is too complicated without providing enough value to be worth it.

quamsta commented 2 years ago

@pyrello

The reasoning behind the initial request was that we had some web applications that we were attempting to integrate with the official version of UIDS and we were noticing collisions with CSS class names that we'd already written, such as hero and some others that I can't remember now. These collisions were essentially making UIDS override styles that we'd already had written or we'd already imported.

It seemed reasonable to request that uids- would be prefixed to any component in UIDS, which would help prevent collisions with class names already written in apps themselves and CSS taken from other frontend frameworks (Bootstrap, Tailwind, Foundation) so that may be used easily in concert with UIDS .

The notion that uids- should be added automatically via a preprocesser seems a bit odd to me, since it could introduce confusion in developers that see a class like, say, bttn in SCSS, but uids-bttn in the generated CSS and HTML markup. In my opinion, it'd just be easier to rewrite and prefix the class names manually in order to be consistent and reduce complexity in preprocessing and generating markup.

pyrello commented 2 years ago

The notion that uids- should be added automatically via a preprocesser seems a bit odd to me, since it could introduce confusion in developers that see a class like, say, bttn in SCSS, but uids-bttn in the generated CSS and HTML markup. In my opinion, it'd just be easier to rewrite and prefix the class names manually in order to be consistent and reduce complexity in preprocessing and generating markup.

The idea for doing this from preprocessing comes in part from observing that libraries like Bootstrap do that: https://github.com/twbs/bootstrap/blob/main/scss/_card.scss. The idea in Bootstrap is that you can just consume their CSS files, where everything is prefixed, but if you want to use their SCSS as the basis of your own SCSS, they are not forcing you to adopt prefixed names for all of their classes.

It also reflects the reality that by the time the request was made, the process of converting all of UIDS over manually to using a statically defined prefix would have required a significant investment to rewrite all the SCSS. It also means that we would have to completely rewrite our integration in SiteNow, where we process the SCSS files ourselves rather than consuming the CSS files. Based on that, preprocessing the changes in seemed like a pretty reasonable middle ground for accommodating this request.

quamsta commented 2 years ago

Unless I'm misunderstanding, it looks like Bootstrap prefixes their CSS variable names in that link to prevent variable collisions but not necessarily their CSS class names:

Prefix Most CSS variables use a prefix to avoid collisions with your own codebase. This prefix is in addition to the -- that’s required on every CSS variable.

Customize the prefix via the $prefix Sass variable. By default, it’s set to bs- (note the trailing dash).

Regardless, as long as UIDS can play nicely with other component libraries by namespacing itself in some way, I'm not that concerned about the methodology of doing it. It just seemed easier to not assume that developers on other platforms are going to build and preprocess the SCSS the same way and to just make a classes that have the namespacing baked-in.

Is the assumption that devs trying to use UIDS are going to use the compiled/generated CSS in its entirety instead of say, picking out components and loading them via SASS and compiling it themselves? e.g.,

instead of

@import "node_modules/uids/src/assets/scss/reset.scss";
@import "node_modules/uids/src/components/logo/logo.scss";
@import "node_modules/uids/src/components/brand-bar/brand-bar.scss";

We want them to import the preprocessed CSS file?

@import "node_modules/uids/dist/uids.css" /* or something similar */
joewhitsitt commented 2 years ago

won't the preprocessed CSS also have component specific versions and not the combined single file?

i expected a considerable investment in re-writing our integration with SiteNow, uids_base, but if every was uids-prefixed we would also have to run update hooks on all sites to find and replace hardcoded classes within wysiwyg markup and such. i'd vote to avoid that. i am not sure where uids adoption is at this point, but I imagine there would be several apps that would not be able to adopt the new version for a long time development time to work on them is cost-recovery. that is one of the reasons why brand adoption across campus has been slow.

bspeare commented 2 years ago

I think that UIDS began with aspirations of being a design system, however, based on our level of staffing and the work required to create a usable design system, I think the best that we can offer is a component library which is why we are shifting to the more developer focused Storybook for 4.x.

I don't think we have the capacity to completely update our usage in SiteNow to work with prefixes. I think it is fine if we want to offer prefixed versions for our header and footer through something like https://github.com/uiowa/uids/issues/191#issuecomment-1152582015, but at this point I don't think we should pursue this for 4.x.

GaryRidgway commented 2 years ago

I think i agree with the sentiment that this might be too heavy a load. @joewhitsitt brings up a good point for needing to do complicated update hooks to replace classes, and i just don't think we have the time to add this to 4.x right now.

quamsta commented 2 years ago

I think that's fair.

I still believe providing at least an automated preprocesser-added-prefix set of components via prefixed CSS file(s) might be a nice-to-have for app devs in the future and would encourage the use of the official components in things like the HR portal, but our group specifically in Student Life doesn't really need for individually prefixed components any longer since we moved to SiteNow and VueJs.

If the need/desire to reduce duplication of effort on campus (namely separate-but-similar component libraries developed by other groups on campus like AIS Components) ever resurfaces, I think we can revisit this discussion. But I've given my two cents on this from an outside-looking-in perspective and I'm fine with whatever ya'll decide here.