devcollaborative / drupal8-devcollab-lightship

This project is now maintained on Drupal.org. A starter theme for Drupal 8, a subtheme of the core base theme Classy. It has Sass and the minimum number of gulp tasks so as to be long-term sustainable, light, accessible, and ready to customize.
https://www.drupal.org/project/lightship
1 stars 0 forks source link

Feedback from Candice Dexter #1

Closed jbates closed 5 years ago

jbates commented 5 years ago

I really like the slimmed down gulp file! The one thing I'd like to see is something to make debugging easier with sass (eg. sourcemaps) just to get that in there and standardize debugging for all your developers.

That's next on my list to add. πŸ‘

Feedback templates: region.html.twig -You have a comment that should be commented out with {# #}.

js: custom.js -You have 'Code for humbergur menu', but it's a general js file (for now).

Thank you for catching those bugs!

sass: custom.scss -There's no 'styles/footer' or 'styles/content-components' right now so those partials should be added or removed from custom.scss.

These are fixed in real life, this is a duplicate repo and it's a bit behind.

Suggestions: -Add a preprocess html function in .theme for setting the node id (ideally this would never be used for theming, but as always there's always a situation where a one-off change is needed).

Yes, great suggestion, this is also on the list for adding to the starter theme.

-Depending on your browser support you might want to add x-ua-compatible tags to the preprocess function above.

Browser support varies based on particular client data, see below. (Edited to add: I meant to say that I like this idea, also.)

Questions: -What breakpoints are you utilizing? Are they standard across projects? Or up to the developer?

Our main designer doesn't do many mobile comps, so we tend to refine in the browser, based on the content, the needs of the particular project, and the developer's judgment. I tend to want to keep breakpoints as few and simple as possible, and add adjustments only where needed. That said, I think we as a team are weak on our implementation of tablet-ish, in-between widths. In part because our clients sometimes don't have budget to refine those too much. But again, that varies among clients.

-What browsers are you supporting? Current and 2 versions behind?

If a client has existing Google Analytics, we use the last 3 months of data to help determine what browsers to focus on. Usually, current FF, Chrome, Safari, IE11, and Edge, plus anything else that shows up a decent amount in traffic stats (i.e. IE9, or Android browsers, etc.).

Right now, we've dropped support for IE8 unless specifically asked to support it. That said, I aim for content that can be read and navigation that can be used in IE8, even if ugly, and maybe in a mobile layout. Social media icons that are crisp SVGs in modern browsers maybe fall back to linked text, rather than PNG versions of the icons, for example. So, it's ideally readable and navigable, but very utilitarian.

Even in IE9-10, we say that layouts and design may be simpler (like 2-columns instead of 4, or whatever). If a client has a lot of IE9 users, for example (this happens, I dunno why!), we may give them a simple float layout behind a modernizr class, where modern browsers get flexbox. If I use grid, I have been using it as an enhancement, inside an @supports.

This was rambling, sorry, but the basic idea is that we try to convey to clients that their web design is not going to be pixel perfect identical in every browser, and that's by design, that's the way the web works. I like to talk about layout as an enhancement, with the most modern browsers getting the nicest experience, but older browsers will get a decent enough, readable experience, and we can give them a little more love when there's a use case for it based on stats or internal org knowledge of browser use.

Thoughts: Lately I've been pulling my base styles out into a separate folder, that way once they're set up you know where they are/can easily find them: -sass β€”-base β€”β€”-variables β€”β€”β€”-_breakpoints β€”β€”β€”-_colors β€”β€”β€”-_fonts β€”β€”-_accessibility β€”β€”-_mixins β€”β€”-_typography β€”-components β€”β€”-_example-component

I think this works for me as far as organizing, I'm curious to see what you think of this. I've implemented this on the Drupal GovCon theme, where I reorganized things from their 2018 theme so you can see that in action here: https://github.com/Drupal4Gov/Drupal-GovCon-2017/tree/master/docroot/themes/custom/twentynineteen/sass

I have been breaking out my own partials into folders on real actual projects, but hadn't standardized a pattern in this starter yet, and I like yours a lot. I would love to establish a pattern for this. Like many humans, I often struggle with how to name or categorize certain kinds of patterns on a real live living breathing site, and I would love to think this through more with someone.

https://css-tricks.com/the-extend-concept/ at K they also heavily depend upon extends, which I'm on the fence about. I feel like either mixins should be used where you're heavily re-using code, but I haven't been utilizing extend enough to have formed enough of an opinion. Particularly on a large site where I know this can bog down things.

I try to use extends and mixins lightly. I used to swear off extends, but have swung back the other way lately. My mind is open and this is another area I'd love to discuss more in the future, as there are definitely tradeoffs in using Sass abstractions. (For example, I used to nest too deeply, in the early days, ye olden times of Compass, and... yeah I had to learn to chill out on nesting, like we all did!)

https://lukyvj.github.io/family.scss/ Another thing that might be useful to you is this mixin family. I found that I use one or two of these pretty frequently when utilizing grids.

I like these, am bookmarking them, thank you!

jbates commented 5 years ago

Definitely do these

I really like the slimmed down gulp file! The one thing I'd like to see is something to make debugging easier with sass (eg. sourcemaps) just to get that in there and standardize debugging for all your developers.

Please add, Candice. :)

Feedback templates: region.html.twig -You have a comment that should be commented out with {# #}.

js: custom.js -You have 'Code for humbergur menu', but it's a general js file (for now).

Please go ahead and fix these bugs.

Please do any or all of these, if you have time.

Suggestions: -Add a preprocess html function in .theme for setting the node id (ideally this would never be used for theming, but as always there's always a situation where a one-off change is needed).

-Depending on your browser support you might want to add x-ua-compatible tags to the preprocess function above.

And better organization of partials, if you're inspired. I liked your suggested structure.

Lately I've been pulling my base styles out into a separate folder, that way once they're set up you know where they are/can easily find them: -sass β€”-base β€”β€”-variables β€”β€”β€”-_breakpoints β€”β€”β€”-_colors β€”β€”β€”-_fonts β€”β€”-_accessibility β€”β€”-_mixins β€”β€”-_typography β€”-components β€”β€”-_example-component

CNDexter commented 5 years ago

See the PR here: https://github.com/devcollaborative/devcollab-theme/pull/2

CNDexter commented 5 years ago

Merged. Closing!