Consolidate clean-theme class naming and organization

Apply clean-theme class to "root" elements of sidebar
and annotator. Remove clean-theme logic from sidebar components, and
instead use selector in SASS modules to apply relevant styles.

Fixes https://github.com/hypothesis/client/issues/2617

src/annotator/sidebar.js

@@ -116,7 +116,7 @@ export default class Sidebar extends Guest {
       frame.className = 'annotator-frame annotator-outer';
 
       if (config.theme === 'clean') {
-        frame.classList.add('annotator-frame--drop-shadow-enabled');
+        frame.classList.add('annotator-frame--theme-clean');
       }
 
       element.appendChild(frame);

src/annotator/test/sidebar-test.js

@@ -103,10 +103,10 @@ describe('Sidebar', () => {
       assert.equal(sidebar.frame.style.display, 'none');
     });
 
-    it('has a shadow if the clean theme is enabled', () => {
+    it('applies a style if theme is configured as "clean"', () => {
       const sidebar = createSidebar({ theme: 'clean' });
       assert.isTrue(
-        sidebar.frame.classList.contains('annotator-frame--drop-shadow-enabled')
+        sidebar.frame.classList.contains('annotator-frame--theme-clean')
       );
     });
 

src/sidebar/components/hypothesis-app.js

+import classnames from 'classnames';
 import { createElement } from 'preact';
 import { useEffect, useMemo } from 'preact/hooks';
 import propTypes from 'prop-types';
@@ -98,6 +99,7 @@ function HypothesisApp({
     () => applyTheme(['appBackgroundColor'], settings),
     [settings]
   );
+  const isThemeClean = settings.theme === 'clean';
 
   const isSidebar = route === 'sidebar';
 
@@ -170,7 +172,9 @@ function HypothesisApp({
 
   return (
     <div
-      className="hypothesis-app js-thread-list-scroll-root"
+      className={classnames('hypothesis-app', 'js-thread-list-scroll-root', {
+        'theme-clean': isThemeClean,
+      })}
       style={backgroundStyle}
     >
       <TopBar

src/sidebar/components/selection-tabs.js

@@ -104,8 +104,6 @@ function SelectionTabs({ isLoading, settings }) {
     selectTab: store.selectTab,
   }));
 
-  const isThemeClean = settings.theme === 'clean';
-
   const selectTab = tabId => {
     store.clearSelection();
     store.selectTab(tabId);
@@ -121,12 +119,7 @@ function SelectionTabs({ isLoading, settings }) {
 
   return (
     <div className="selection-tabs-container">
-      <div
-        className={classnames('selection-tabs', {
-          'selection-tabs--theme-clean': isThemeClean,
-        })}
-        role="tablist"
-      >
+      <div className="selection-tabs" role="tablist">
         <Tab
           count={annotationCount}
           isWaitingToAnchor={isWaitingToAnchorAnnotations}

src/sidebar/components/test/hypothesis-app-test.js

@@ -429,14 +429,34 @@ describe('HypothesisApp', () => {
     });
   });
 
-  it('applies theme config', () => {
-    const style = { backgroundColor: 'red' };
-    fakeApplyTheme.returns({ backgroundColor: 'red' });
+  describe('theming', () => {
+    it('applies theme config', () => {
+      const style = { backgroundColor: 'red' };
+      fakeApplyTheme.returns({ backgroundColor: 'red' });
+
+      const wrapper = createComponent();
+      const background = wrapper.find('.hypothesis-app');
+
+      assert.calledWith(fakeApplyTheme, ['appBackgroundColor'], fakeSettings);
+      assert.deepEqual(background.prop('style'), style);
+    });
+  });
+
+  it('applies a clean-theme style when config sets theme to "clean"', () => {
+    fakeSettings.theme = 'clean';
+
+    const wrapper = createComponent();
+    const container = wrapper.find('.hypothesis-app');
+
+    assert.isTrue(container.hasClass('theme-clean'));
+  });
+
+  it('does not apply clean-theme style when config does not assert `clean` theme', () => {
+    fakeSettings.theme = '';
 
     const wrapper = createComponent();
-    const background = wrapper.find('.hypothesis-app');
+    const container = wrapper.find('.hypothesis-app');
 
-    assert.calledWith(fakeApplyTheme, ['appBackgroundColor'], fakeSettings);
-    assert.deepEqual(background.prop('style'), style);
+    assert.isFalse(container.hasClass('hypothesis-app--theme-clean'));
   });
 });

src/sidebar/components/test/selection-tabs-test.js

@@ -134,17 +134,6 @@ describe('SelectionTabs', function () {
       assert.equal(tabs.at(1).prop('title'), 'Page notes');
     });
 
-    it('should show the clean theme when settings contains the clean theme option', function () {
-      fakeSettings.theme = 'clean';
-      const wrapper = createComponent();
-      assert.isTrue(wrapper.exists('.selection-tabs--theme-clean'));
-    });
-
-    it('should not show the clean theme when settings does not contain the clean theme option', function () {
-      const wrapper = createComponent();
-      assert.isFalse(wrapper.exists('.selection-tabs--theme-clean'));
-    });
-
     it('should not display the new-note-btn when the annotations tab is active', function () {
       const wrapper = createComponent();
       assert.equal(wrapper.find('NewNoteButton').length, 0);

src/sidebar/components/test/thread-card-test.js

@@ -15,12 +15,7 @@ describe('ThreadCard', () => {
 
   function createComponent(props) {
     return mount(
-      <ThreadCard
-        frameSync={fakeFrameSync}
-        settings={{}}
-        thread={fakeThread}
-        {...props}
-      />
+      <ThreadCard frameSync={fakeFrameSync} thread={fakeThread} {...props} />
     );
   }
 
@@ -64,12 +59,6 @@ describe('ThreadCard', () => {
     assert(wrapper.find('.thread-card').hasClass('is-focused'));
   });
 
-  it('applies a CSS class if settings indicate a `clean` theme', () => {
-    const wrapper = createComponent({ settings: { theme: 'clean' } });
-
-    assert(wrapper.find('.thread-card').hasClass('thread-card--theme-clean'));
-  });
-
   it('shows document info if current route is not sidebar', () => {
     fakeStore.route.returns('whatever');
 

src/sidebar/components/test/top-bar-test.js

@@ -249,12 +249,6 @@ describe('TopBar', () => {
     assert.ok(searchInput.exists());
   });
 
-  it('shows the clean theme when settings contains the clean theme option', () => {
-    fakeSettings.theme = 'clean';
-    const wrapper = createTopBar();
-    assert.isTrue(wrapper.exists('.top-bar--theme-clean'));
-  });
-
   context('in the stream and single annotation pages', () => {
     it('does not render the group list, sort menu or share menu', () => {
       const wrapper = createTopBar({ isSidebar: false });

src/sidebar/components/thread-card.js

@@ -17,7 +17,6 @@ import Thread from './thread';
  * @typedef ThreadCardProps
  * @prop {Thread} thread
  * @prop {Object} frameSync - Injected service
- * @prop {MergedConfig} settings - Injected service
  */
 
 /**
@@ -26,7 +25,7 @@ import Thread from './thread';
  *
  * @param {ThreadCardProps} props
  */
-function ThreadCard({ frameSync, settings, thread }) {
+function ThreadCard({ frameSync, thread }) {
   const threadTag = thread.annotation && thread.annotation.$tag;
   const isFocused = useStore(store => store.isAnnotationFocused(threadTag));
   const showDocumentInfo = useStore(store => store.route() !== 'sidebar');
@@ -71,7 +70,6 @@ function ThreadCard({ frameSync, settings, thread }) {
       key={thread.id}
       className={classnames('thread-card', {
         'is-focused': isFocused,
-        'thread-card--theme-clean': settings.theme === 'clean',
       })}
     >
       <Thread thread={thread} showDocumentInfo={showDocumentInfo} />
@@ -82,9 +80,8 @@ function ThreadCard({ frameSync, settings, thread }) {
 ThreadCard.propTypes = {
   thread: propTypes.object.isRequired,
   frameSync: propTypes.object.isRequired,
-  settings: propTypes.object,
 };
 
-ThreadCard.injectedProps = ['frameSync', 'settings'];
+ThreadCard.injectedProps = ['frameSync'];
 
 export default withServices(ThreadCard);

src/sidebar/components/top-bar.js

-import classnames from 'classnames';
 import { Fragment, createElement } from 'preact';
 import propTypes from 'prop-types';
 
@@ -49,7 +48,6 @@ function TopBar({
   settings,
   streamer,
 }) {
-  const useCleanTheme = settings.theme === 'clean';
   const showSharePageButton = !isThirdPartyService(settings);
   const loginLinkStyle = applyTheme(['accentColor'], settings);
 
@@ -112,9 +110,7 @@ function TopBar({
   );
 
   return (
-    <div
-      className={classnames('top-bar', useCleanTheme && 'top-bar--theme-clean')}
-    >
+    <div className="top-bar">
       {/* Single-annotation and stream views. */}
       {!isSidebar && (
         <div className="top-bar__inner content">

src/styles/annotator/annotator.scss

@@ -66,11 +66,6 @@
   }
 }
 
-/** Visible with clean theme */
-.annotator-frame--drop-shadow-enabled {
-  box-shadow: var.$annotator-shadow--sidebar;
-}
-
 .annotator-placeholder {
   opacity: 0;
   position: absolute;
@@ -98,6 +93,11 @@
   z-index: 10000;
 }
 
+/** Affordances for clean theme */
+#{var.$annotator--theme-clean} {
+  box-shadow: var.$annotator-shadow--sidebar;
+}
+
 /*
   Mobile layout
   240-479 px

src/styles/mixins/molecules.scss

@@ -30,7 +30,8 @@
     @include utils.shadow--active;
   }
 
-  &--theme-clean {
+  /** Clean theme affordances */
+  #{var.$sidebar--theme-clean} & {
     // Give a little more space so that the border appears centered
     // between cards
     padding-bottom: 1.5em;

src/styles/sidebar/components/selection-tabs.scss

@@ -24,10 +24,6 @@
   margin: 0 var.$layout-space--xxsmall;
 }
 
-.selection-tabs--theme-clean {
-  margin-left: 15px;
-}
-
 .selection-tabs__type {
   @include buttons.reset-native-btn-styles;
   @include focus.outline-on-keyboard-focus;
@@ -69,3 +65,8 @@
   padding: 2em;
   text-align: center;
 }
+
+/** Clean theme affordances */
+#{var.$sidebar--theme-clean} .selection-tabs {
+  margin-left: 15px;
+}

src/styles/sidebar/components/top-bar.scss

@@ -56,10 +56,6 @@
   }
 }
 
-.top-bar--theme-clean {
-  border-bottom: none;
-}
-
 .top-bar__login-button {
   @include buttons.button;
   @include utils.font--large;
@@ -105,3 +101,8 @@
   margin-right: auto;
   white-space: nowrap;
 }
+
+/** Clean theme affordances */
+#{var.$sidebar--theme-clean} .top-bar {
+  border-bottom: none;
+}

src/styles/variables.scss

@@ -110,6 +110,10 @@ $icon-size--small: 12px;
 $icon-size--medium: 16px;
 $icon-size--large: 24px;
 
+// Selectors for applying clean-theme styling
+$annotator--theme-clean: '.annotator-frame--theme-clean';
+$sidebar--theme-clean: '.theme-clean';
+
 // Annotator-specific values
 // -----------------------------------
 $annotator-bucket-bar-width: 22px;