middlebury / midd-frontend

Frontend code for middlebury_theme
https://midd-frontend.now.sh
GNU General Public License v3.0
0 stars 0 forks source link

Consider a nested select to determine if the breadcrumb is 'light' #58

Open imcbride opened 5 years ago

imcbride commented 5 years ago

The page title uses .page-header--has-image .page-header__title to determine if it should appear in white instead of black, but the breadcrumb requires a local class, breadcrumb--light to make the same determination.

This requires me to add a special check for the breadcrumb to see if it is appearing on a page with a banner image https://github.com/middlebury/drupal8/blob/master/web/themes/custom/middlebury_theme/middlebury_theme.theme#L38 in addition to the check that needs to be performed at the page level https://github.com/middlebury/drupal8/blob/master/web/themes/custom/middlebury_theme/middlebury_theme.theme#L59. Using a nested selector, like the page title uses, would eliminate one of these redundant checks.

zebapy commented 5 years ago

I can probably make this change. I was trying to keep components unaware of any child components but if it saves some database calls just for a class, that seems better. The page header may need a rework because we have some title concerns on mobile but want to see actual content first.

imcbride commented 5 years ago

Sounds good. Fine to leave it as-is for now and wait to see what it looks like with real content, knowing that will change things. It's working, so this can be left as a later optimization.