Restructure CSS

[front-endian]

Summary

While working on U-M's beta interface for our repository based on Sufia 7 we had a lot of trouble working with Sufia's current stylesheets. The following suggestions are centered around simplifying maintenance and making it easier for groups to customize Sufia's views as much or as little as they see fit.

Note: This issue should probably be dealt with as part of implementing whatever the UI working group comes up with instead of working on the current interface.

Suggestions

1. Reduce the amount of CSS in Sufia to a bare minimum, instead relying on native Bootstrap wherever possible.
   • Bootstrap is a great tool that was made to be easier to customize. If we let Bootstrap do almost all the heavy lifting and trust it to have proper padding/margins it will be a lot easier for developers to customize Sufia to meet their needs
   • This is also reduce the amount of CSS which has to be maintained by the Sufia community.
2. Any custom CSS shouldn't have any id's in the selectors, and should only have at most two classes.
   • This will make it easier to overwrite styles and will ease longterm maintenance.
3. Add semantically named classes to most elements in Sufia, even when they aren't being used by stock Sufia's CSS.
   • This will make it easier for people to restyle Sufia without having the go in and edit the HTML.
4. Improve the accessibility of Sufia's stock styles.
   • There are a number of things that need improvement so that it is easier for more people to use Sufia, such as more obvious link styles, clear :hover and :focus actions, etc.

     Related

See #1594 for the start of this discussion See #1608 for the discussion of front-end development best practices

[front-endian]

@grosscol

[jcoyne]

I think I'm pretty strongly against point one. I think that people want a repository that looks good out of the box. Not every institution is Michigan, and has the resources to completely redo the CSS.

[jcoyne]

I think point 4 got messed up (it's trailing point 3)

There are a number of things that need improvement

Please expand on these things. Let's make all suggestions actionable.

[grosscol]

I don't get the impression that many institutions that are not already in the mix will be doing a great deal of work in house. I imagine that the hydra is a box project will provide a very stylish styling.

For the base Sufia, making easier to modify css should give us a more stable (fewer places to touch) base for UI work.

I do concur with a list of actionable items.

[jcoyne]

@grosscol But what about the institutions that are already in the mix. Those are the ones I care most about right now.

[grosscol]

My assumption was that the existing members have already modified the views to suite their own requirements. So long as we don't alter the routes and helpers the existing modified views should still function. Worst case is copying up views from older versions.

[front-endian]

On point 1: I think Sufia can be made to look great out of the box and use less custom CSS at the same time. Blacklight looks really good, and it uses barely any CSS, mostly just supplementing Bootstrap where needed with some generic classes. Bootstrap is designed to look good and contemporary out of the box.

[awead]

I'm against point 1 as well because Sufia needs to have a "look" to it, but I agree that it should be simple. I think the other points are good, especially 2 and 3. I think we could create a default Sufia skin that implements all the various bits that are a part of 2 and 3, then the user can either remove that and create their own, or tweak it. This way there's at least a kind of a template for them to work with. I think that balances institutions that have UI resources versus those that don't.

I can't speak to 4 without more information.

[carolyncole]

@justcolin @jcoyne I wonder if we could accomplish more customization in the css by generating the css into the app instead of hiding it within sufia. I think it would be great to use the two perspectives to allow for customization to be done more easily in Sufia!

[awead]

@cam156 yes, that's what I was thinking about with my "skin" comment. The generated CSS would use the classes and selectors from points 2 and 3.

[front-endian]

@cam156 I like the idea of generating the CSS into the app, or providing a gem which creates the skinned version, as long as the unskinned version still works and looks acceptable.

[awead]

@justcolin I think the point of the generated CSS is that it would be the default look. There wouldn't be an "unskinned" version. Otherwise, you're maintaining two different looks, the default skin and the unskinned. Just have one default look that's generated into the application and can be overridden easily. It also goes to what @jcoyne and I were saying that Sufia needs a polished look out of the box.

[front-endian]

@awead I am not so sure about that. If we go with the "use Bootstrap as much as possible for layout" idea it wouldn't be any more work to have Sufia look okay without that generated CSS. If a couple of things are broken, that's fine, but encouraging the site to be mostly passable without the generated CSS would encourage better use of Bootstrap to get most of the work done. That would also the go towards simplifying the generated CSS. This doesn't mean that Sufia wouldn't look great out of the box.

[awead]

Again, this goes back to point 1: Sufia needs to look good out of the box. To do that, we need to test the complete application, including its generated CSS, otherwise we don't know the quality of the finished product. The CSS can be generated as part of the install process, but can be overridden. If the user wants to delete that generated file and use the "skinless" version, they're free to do that, but I don't see the point in having to support that scenario in our automated testing. The generated CSS can be as simple as possible, but it needs to be there, in my mind, because that's what the full application is.

[jcoyne]

If you want an unskinned version, can't you just remove the generated sufia lines from application.css in your local install?

[front-endian]

@awead: I believe we are in agreement. I don't think the automated testing should be tested without the the generated CSS. However, I do think that better practices would be encouraged if while working on the next UI we tried to get as far as we can without using any generated CSS. It would help everyone have an easier time.

@jcoyne: You can, but it destroys a lot of the layout, causing one to have to do a lot of extra work. It is possible to use Bootstrap more for doing layout work, reduce the dependency on custom CSS, while still having a fantastic looking website and making it easier for people trying to customize Sufia in small or large amounts.

[front-endian]

@jcoyne: My goal in these suggestions isn't to make things worse for people using Sufia out of the box while making it easier for people who want to customize the website. I want to make sure both parties have a better experience.

[awead]

So now I'm asking myself: what does done look like? Currently, Sufia's "skin" is: https://github.com/projecthydra/sufia/blob/master/app/assets/stylesheets/sufia.css.scss Which, in looking at, doesn't look too bad to me. The only thing I'd like to see is what I mentioned in the TODO... use import exclusively and not require. Or have the requires somewhere else so it's explicit what parts are using asset pipeline and what parts are using Sass.

It seems to me we're talking about restructuring parts of the imported css (settings, header, styles, etc.) and perhaps floating them up into the sufia.css file where they can be overridden there. However, the current strategy still looks pretty good to me: I simply add one line to my application.css file and be done. Or, I can copy sufia.css into my project and make selective overrides directly in that file, or I can make more large-scale changes by not importing some of the files and substituting my own.

So I guess what I'm getting at is, what don't we like about the current process, and what are we trying to change about it?

[front-endian]

@awead If all SASS files are partials imported into sufis.css.scss then that is good enough (that wasn't part of the original issue). See the Suggestions section of this issue for my main suggestions.

[front-endian]

@jcoyne: I strongly believe that more use of Bootstrap will not make Sufia look worse out of the box. Arguably it will make it look a lot better.

If Bootstrap is used for all or almost all of the layout we will end up with a robust, easier to maintain design. Then we can overwrite some of Boostrap's reset to customize text and/or UI elements to keep compatibility with Bootstraps classes, but make it our own. Finally, with a few custom rules to add a little more branding you can end up with a distinctive, modern, robust design that looks great, and has less code making it easier to maintain & alter.

[jcoyne]

@justcolin I'll keep an open mind while I eagerly await a PR.

[front-endian]

@jcoyne I assume you don't want any pull requests for the current interface, correct (since large portions are about to be rewritten anyways)?

[jcoyne]

As far as I know, only the upload / create a new work is changing. I'm not the person who is most involved in the Sufia plan though.

[front-endian]

Okay; once the wireframes are set and implementation planning begins I will investigate updating the pages that aren't being touched by that work.

[mjgiarlo]

@justcolin Yes, it may be easier to work on this once all the UI-related issues on the 7.0.0 release are completed.

[mtribone]

@justcolin @jcoyne @mjgiarlo The UI working group is proposing add and edit new work, new work show page, a new top bar, and may touch things like the tabbed my works, my collections, etc. and the dashboard.

[jcoyne]

@mtribone What's the projected timeline for completing that work?

[mtribone]

@jcoyne We have most of the issues in Github now prefaced with UI. Anyone can pick up the following issues and get crackin' and/or give us feedback and/or ask questions. Right now we are missing the show page issue because we are still discussing the layout and hierachy.

UI Create _save_work_widget partial https://github.com/projecthydra/sufia/issues/1562 UI Create _find_work_widget partial https://github.com/projecthydra/sufia/issues/1563 UI Create _find_collection_widget partial https://github.com/projecthydra/sufia/issues/1572 UI create _share_tab partial https://github.com/projecthydra/sufia/issues/1573 UI Create _relationships_tab partial https://github.com/projecthydra/sufia/issues/1577 UI Create _metadata_tab partial https://github.com/projecthydra/sufia/issues/1578 UI Create _toolbar_widget https://github.com/projecthydra/sufia/issues/1583 UI Create _files_tab partial https://github.com/projecthydra/sufia/issues/1591

[jcoyne]

@mtribone :+1:

[mjgiarlo]

@justcolin Given that a Hydra Dev Congress event in Ann Arbor was recently announced, I wonder whether some of the work that you ticketed here might be good candidates for a multi-person face-to-face hack in May. The timing on that would be good, I believe, and that way there's a captive audience and there are likely to be multiple Sufia-using folks in the room.

[awead]

@mjgiarlo good idea... maybe we can send @mtribone as our delegate, provided the PA legislature gets their act together.

[mjgiarlo]

@awead :+1:

@justcolin Might that timing work for your purposes too?

[front-endian]

@mjgiarlo: That could work well for us, I'll just have to check with the higher ups.

[mtribone]

@mjgiarlo Yeah that would fantastic to use the upcoming dev meeting in May! But yes, sadly we have no idea when the travel ban will be lifted. However, this work is operationally critical...

[front-endian]

@mjgiarlo, @mtribone and @jcoyne: Ignore this, see later comment for why.

Original comment: Some of the issues we encountered were related to odd or overly specific styles in Curation Concerns. Would you all be open to removing the Curation Concerns CSS from Sufia so they are decoupled, or would you rather fix the CSS in Curation Concerns as well?

There isn't much, but it is just enough to cause some issue.

[jcoyne]

I'm a bit surprised that you are using any CurationConcerns styles in sufia. I would think that those aren't required.

[front-endian]

Scratch that, I was just looking at it again and it looks like Bootstrap's CSS is being included in a file called curation_concerns when you load the site in dev mode. The styles we were fighting were just unusual uses of Bootstrap. Sorry!

[front-endian]

@jcoyne: What's the reasoning for closing this ticket? I am not opposed to it, just wondering what the reasoning is.

[mjgiarlo]

I'm guessing it was a misfire. It's re-opened now, @justcolin.

[jcoyne]

Oops. I must have hit it by mistake

[mjgiarlo]

GitHub makes it too easy to click "Close and comment!"

[front-endian]

It must be to promote faster development. Doesn't matter if the work actually gets done, just matters if tickets are getting closed. :-P

[mjgiarlo]

Ha! #worksforme #wontfix ;)

[front-endian]

I suggest that we use BEM for the CSS restructure. See this comment for the reasoning, and keep the discussing on #1608.

[mtribone]

@justcolin Sounds good. Yeah, I've been researching BEM a little bit and agree that its syntax is a little ugly. However, if it will do the job and help other people fix/customize the code I'm all for it. I might start playing around with it locally and refactor some views.

[mjgiarlo]

@mtribone @justcolin @grosscol @awead @cam156 See the PRs immediately above this comment. This work is starting to happen in bite-sized chunks.

[mtribone]

@mjgiarlo Yeah I see that happening. I was experimenting with BEM and the masthead, but @cbeer is refactoring a lot of stuff. So Imma gonna wait and work on something else in the meantime while things shake out. Are we still interested in using BEM on top of Bootstrap?

[mjgiarlo]

@mtribone What do you think, @cbeer @jcoyne? I'm not familiar enough with BEM to comment at the moment. It would be good to come up with a plan so we don't step on each other's toes!

[cbeer]

I'd guess doing BEM will be easier if we clean things up first.

[front-endian]

@mjgiarlo, @mjgiarlo, @jcoyne and @cbeer: Even if we only use Bootstrap and manage to remove all the custom CSS, adding BEM style classes to everything would still be a huge help. It would make customization easier for everyone (no need to edit templates if a class is already there), and if we ever need to add custom CSS in the future we will already have a class there already.

If you don't use BEM, that's fine, it just provides a defined and easy to understand way to name classes. The most important thing to me is to remove all id's for styling (id's for JS are fine) and give almost everything a class instead.