Fix for details button misbehaving on mobile (#1114)

* chore(make): convenience commands for installation & dev server * docs: easier development env instructions * docs: add note about docker volumes * docs: detail pre-commit configuration * style: eslint and prettier changes * Allow for passing custom classes to fade-in * style: eslint & prettier for the details button component * style: react lifecycle methods to come first * docs: add instructions to disable pre-commit * style: devices components * Move render methods to the last (together) in the order list * Remove unused component import * React lifecycle to come first before our own methods * General styling and eslint changes * Cleaner renderContent method using switch/case (fixes an eslint error as well!) * Cleaner renderPill method with proper spacing + removing uncessary else * style: more eslint/prettier for pages components * Use newer Fragment syntax * Remove unnecessary else statement * Use backtick strings for concatenating strings * Remove unnecessary space * Remove unused imports and variable declarations * Bunch render methods together as last in the order list * fix: details button to drop to the bottom naturally on smaller screens This _mostly_ fixes one of the issues being tracked on #972, titled "Details button issue on Firefox specifically" * refactor: reduce usage of our CSS class in favor of tailwind's util classes * refactor: remove our css classes in favor of Tailwind's util classes
plausible · Jun 15, 2021 · c95d375 · c95d375
1 parent 6450692
commit c95d375
Show file tree

Hide file tree

Showing 18 changed files with 261 additions and 126 deletions.
diff --git a/.tool-versions b/.tool-versions
@@ -1,3 +1,4 @@
 elixir 1.11.3-otp-23
 erlang 23.2.1
 nodejs 15.3.0
+python 3.9.4
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
@@ -9,12 +9,13 @@ Make sure Docker, Elixir, Erlang and Node.js are all installed on your developme
 ### Start the environment:
 
 1. Run both `make postgres` and `make clickhouse`.
-2. Run `mix deps.get`. This will download the required Elixir dependencies.
-2. Run `mix ecto.create`. This will create the required databases in both Postgres and Clickhouse.
-3. Run `mix ecto.migrate` to build the database schema.
-4. Run `npm ci --prefix assets` to install the required node dependencies.
-5. Run `mix phx.server` to start the Phoenix server.
-6. The system is now available on `localhost:8000`.
+2. You can then get set up with the following bits in one go with `make install`.
+  1. Run `mix deps.get`. This will download the required Elixir dependencies.
+  2. Run `mix ecto.create`. This will create the required databases in both Postgres and Clickhouse.
+  3. Run `mix ecto.migrate` to build the database schema.
+  4. Run `npm ci --prefix assets` to install the required node dependencies.
+3. Run `make server` or `mix phx.server` to start the Phoenix server.
+4. The system is now available on `localhost:8000`.
 
 ### Creating an account
 
@@ -29,4 +30,10 @@ Make sure Docker, Elixir, Erlang and Node.js are all installed on your developme
 1. Stop and remove the Postgres container with `make postgres-stop`.
 2. Stop and remove the Clickhouse container with `make clickhouse-stop`.
 
-Volumes are preserved. You'll find that the Postgres and Clickhouse state are retained when you bring them up again the next time: no need to re-register and so on.
+Volumes are preserved. You'll find that the Postgres and Clickhouse state are retained when you bring them up again the next time: no need to re-register and so on.
+
+Note: Since we are deleting the containers, be careful when deleting volumes with `docker volume prune`. You might accidentally delete the database and would have to go through re-registration process.
+
+### Pre-commit hooks
+
+`pre-commit` requires Python to be available locally and covers JavaScript and CSS. Set up with `pip install --user pre-commit` followed by `pre-commit install`. Conversely, if the prompts are far too bothersome, remove with `pre-commit uninstall`.
diff --git a/Makefile b/Makefile
@@ -1,3 +1,12 @@
+install:
+	mix deps.get
+	mix ecto.create
+	mix ecto.migrate
+	npm ci --prefix assets
+
+server:
+	mix phx.server
+
 clickhouse:
 	docker run --detach -p 8123:8123 --ulimit nofile=262144:262144 --volume=$$PWD/.clickhouse_db_vol:/var/lib/clickhouse --name plausible_clickhouse yandex/clickhouse-server:21.3.2.5
 

diff --git a/assets/css/app.css b/assets/css/app.css
@@ -247,8 +247,7 @@ blockquote {
 }
 
 .stats-item {
-  @apply mt-6;
-  width: 100%;
+  min-height: 436px;
 }
 
 @screen md {
@@ -257,6 +256,12 @@ blockquote {
     margin-right: 6px;
     width: calc(50% - 6px);
     position: relative;
+    min-height: initial;
+    height: 436px;
+  }
+
+  .stats-item__header {
+    height: inherit;
   }
 }
 

diff --git a/assets/js/dashboard/fade-in.js b/assets/js/dashboard/fade-in.js
@@ -1,8 +1,12 @@
 import React from 'react';
 
-export default function FadeIn({show, children}) {
-  const className = show ? "fade-enter-active" : "fade-enter"
-
-  return <div className={className}>{children}</div>
+export default function FadeIn({className, show, children}) {
+  return (
+    <div
+      className={`${className || ''} ${show ? "fade-enter-active" : "fade-enter"}`}
+    >
+      {children}
+    </div>
+)
 }
 
diff --git a/assets/js/dashboard/lazy-loader.js b/assets/js/dashboard/lazy-loader.js
@@ -1,10 +1,6 @@
 import React from 'react';
 
 export default class extends React.Component {
-  constructor(props) {
-    super(props)
-  }
-
   componentDidMount() {
     this.observer = new IntersectionObserver((entries) => {
       if (entries[0].isIntersecting) {
@@ -24,7 +20,13 @@ export default class extends React.Component {
 
   render() {
     return (
-      <div ref={(el) => this.element = el} className={this.props.className} style={this.props.style}>{this.props.children}</div>
+      <div
+        ref={(el) => { this.element = el }}
+        className={this.props.className}
+        style={this.props.style}
+      >
+        {this.props.children}
+      </div>
     );
   }
 }
diff --git a/assets/js/dashboard/stats/countries.js b/assets/js/dashboard/stats/countries.js
@@ -148,7 +148,9 @@ class Countries extends React.Component {
 
   render() {
     return (
-      <div className="relative p-4 bg-white rounded shadow-xl stats-item dark:bg-gray-825" style={{height: '436px'}}>
+      <div
+        className="relative p-4 bg-white rounded shadow-xl stats-item flex flex-col dark:bg-gray-825 mt-6 w-full"
+      >
         <LazyLoader onVisible={this.onVisible}>
           { this.state.loading && <div className="mx-auto my-32 loading"><div></div></div> }
           <FadeIn show={!this.state.loading}>

diff --git a/assets/js/dashboard/stats/devices/browsers.js b/assets/js/dashboard/stats/devices/browsers.js
@@ -14,17 +14,17 @@ export default class Browsers extends React.Component {
     this.onVisible = this.onVisible.bind(this)
   }
 
-  onVisible() {
-    this.fetchBrowsers()
-    if (this.props.timer) this.props.timer.onTick(this.fetchBrowsers.bind(this))
-  }
-
   componentDidUpdate(prevProps) {
     if (this.props.query !== prevProps.query) {
       this.setState({loading: true, browsers: null})
       this.fetchBrowsers()
     }
   }
+
+  onVisible() {
+    this.fetchBrowsers()
+    if (this.props.timer) this.props.timer.onTick(this.fetchBrowsers.bind(this))
+  }
 
   fetchBrowsers() {
     if (this.props.query.filters.browser) {
@@ -84,9 +84,9 @@ export default class Browsers extends React.Component {
 
   render() {
     return (
-      <LazyLoader onVisible={this.onVisible}>
+      <LazyLoader onVisible={this.onVisible} className="flex flex-col flex-grow">
         { this.state.loading && <div className="mx-auto loading mt-44"><div></div></div> }
-        <FadeIn show={!this.state.loading}>
+        <FadeIn show={!this.state.loading} className="flex-grow">
           { this.renderList() }
         </FadeIn>
       </LazyLoader>

diff --git a/assets/js/dashboard/stats/devices/index.js b/assets/js/dashboard/stats/devices/index.js
@@ -8,7 +8,6 @@ import OperatingSystems from './operating-systems'
 import FadeIn from '../../fade-in'
 import numberFormatter from '../../number-formatter'
 import Bar from '../bar'
-import MoreLink from '../more-link'
 import * as api from '../../api'
 
 
@@ -46,67 +45,91 @@ class ScreenSizes extends React.Component {
     this.onVisible = this.onVisible.bind(this)
   }
 
-  onVisible() {
-    this.fetchScreenSizes()
-    if (this.props.timer) this.props.timer.onTick(this.fetchScreenSizes.bind(this))
-  }
-
   componentDidUpdate(prevProps) {
     if (this.props.query !== prevProps.query) {
       this.setState({loading: true, sizes: null})
       this.fetchScreenSizes()
     }
   }
 
+  onVisible() {
+    this.fetchScreenSizes()
+    if (this.props.timer) this.props.timer.onTick(this.fetchScreenSizes.bind(this))
+  }
+
+
   fetchScreenSizes() {
-    api.get(`/api/stats/${encodeURIComponent(this.props.site.domain)}/screen-sizes`, this.props.query)
+    api.get(
+      `/api/stats/${encodeURIComponent(this.props.site.domain)}/screen-sizes`,
+      this.props.query
+    )
       .then((res) => this.setState({loading: false, sizes: res}))
   }
 
+  label() {
+    return this.props.query.period === 'realtime' ? 'Current visitors' : 'Visitors'
+  }
+
   renderScreenSize(size) {
     const query = new URLSearchParams(window.location.search)
     query.set('screen', size.name)
 
     return (
       <div className="flex items-center justify-between my-1 text-sm" key={size.name}>
         <div className="w-full h-8" style={{maxWidth: 'calc(100% - 6rem)'}}>
-          <Bar count={size.count} all={this.state.sizes} bg="bg-green-50 dark:bg-gray-500 dark:bg-opacity-15" />
-          <span tooltip={EXPLANATION[size.name]} className="flex px-2 dark:text-gray-300" style={{marginTop: '-26px'}} >
+          <Bar
+            count={size.count}
+            all={this.state.sizes}
+            bg="bg-green-50 dark:bg-gray-500 dark:bg-opacity-15"
+          />
+          <span
+            tooltip={EXPLANATION[size.name]}
+            className="flex px-2 dark:text-gray-300"
+            style={{marginTop: '-26px'}}
+          >
             <Link className="block truncate hover:underline" to={{search: query.toString()}}>
               {iconFor(size.name)} {size.name}
             </Link>
           </span>
         </div>
-        <span className="font-medium dark:text-gray-200">{numberFormatter(size.count)} <span className="inline-block w-8 text-xs text-right">({size.percentage}%)</span></span>
+        <span
+          className="font-medium dark:text-gray-200"
+        >
+          {numberFormatter(size.count)}
+          <span className="inline-block w-8 text-xs text-right">({size.percentage}%)</span>
+        </span>
       </div>
     )
   }
 
-  label() {
-    return this.props.query.period === 'realtime' ? 'Current visitors' : 'Visitors'
-  }
-
   renderList() {
     if (this.state.sizes && this.state.sizes.length > 0) {
       return (
         <React.Fragment>
-          <div className="flex items-center justify-between mt-3 mb-2 text-xs font-bold tracking-wide text-gray-500">
+          <div
+            className="flex items-center justify-between mt-3 mb-2 text-xs font-bold tracking-wide text-gray-500"
+          >
             <span>Screen size</span>
             <span>{ this.label() }</span>
           </div>
           { this.state.sizes && this.state.sizes.map(this.renderScreenSize.bind(this)) }
         </React.Fragment>
       )
-    } else {
-      return <div className="font-medium text-center text-gray-500 mt-44 dark:text-gray-400">No data yet</div>
     }
+    return (
+      <div
+        className="font-medium text-center text-gray-500 mt-44 dark:text-gray-400"
+      >
+        No data yet
+      </div>
+    )
   }
 
   render() {
     return (
-      <LazyLoader onVisible={this.onVisible}>
+      <LazyLoader onVisible={this.onVisible} className="flex flex-col flex-grow">
         { this.state.loading && <div className="mx-auto loading mt-44"><div></div></div> }
-        <FadeIn show={!this.state.loading}>
+        <FadeIn show={!this.state.loading} class="flex-grow">
           { this.renderList() }
         </FadeIn>
       </LazyLoader>
@@ -117,57 +140,85 @@ class ScreenSizes extends React.Component {
 export default class Devices extends React.Component {
   constructor(props) {
     super(props)
-    this.tabKey = 'deviceTab__' + props.site.domain
+    this.tabKey = `deviceTab__${  props.site.domain}`
     const storedTab = storage.getItem(this.tabKey)
     this.state = {
       mode: storedTab || 'size'
     }
   }
 
-  renderContent() {
-    if (this.state.mode === 'size') {
-      return <ScreenSizes site={this.props.site} query={this.props.query} timer={this.props.timer} />
-    } else if (this.state.mode === 'browser') {
-      return <Browsers site={this.props.site} query={this.props.query} timer={this.props.timer} />
-    } else if (this.state.mode === 'os') {
-      return <OperatingSystems site={this.props.site} query={this.props.query} timer={this.props.timer} />
-    }
-  }
-
+
   setMode(mode) {
     return () => {
       storage.setItem(this.tabKey, mode)
       this.setState({mode})
     }
   }
+
+  renderContent() {
+    switch (this.state.mode) {
+      case 'browser':
+        return <Browsers site={this.props.site} query={this.props.query} timer={this.props.timer} />
+      case 'os':
+        return (
+          <OperatingSystems
+            site={this.props.site}
+            query={this.props.query}
+            timer={this.props.timer}
+          />
+        )
+      case 'size':
+      default:
+        return (
+          <ScreenSizes
+            site={this.props.site}
+            query={this.props.query}
+            timer={this.props.timer}
+          />
+        )
+    }
+  }
 
   renderPill(name, mode) {
     const isActive = this.state.mode === mode
 
     if (isActive) {
-      return <li className="inline-block h-5 font-bold text-indigo-700 border-b-2 border-indigo-700 dark:text-indigo-500 dark:border-indigo-500">{name}</li>
-    } else {
-      return <li className="cursor-pointer hover:text-indigo-600" onClick={this.setMode(mode)}>{name}</li>
+      return (
+        <li
+          className="inline-block h-5 font-bold text-indigo-700 border-b-2 border-indigo-700 dark:text-indigo-500 dark:border-indigo-500"
+        >
+          {name}
+        </li>
+      )
     }
+
+    return (
+      <li
+        className="cursor-pointer hover:text-indigo-600"
+        onClick={this.setMode(mode)}
+      >
+        {name}
+      </li>
+    )
   }
 
   render() {
     return (
-      <div className="stats-item">
-        <div className="relative p-4 bg-white rounded shadow-xl dark:bg-gray-825" style={{height: '436px'}}>
-
+      <div
+        className="stats-item flex flex-col mt-6 stats-item--has-header w-full"
+      >
+        <div
+          className="stats-item__header flex flex-col flex-grow relative p-4 bg-white rounded shadow-xl dark:bg-gray-825"
+        >
           <div className="flex justify-between w-full">
             <h3 className="font-bold dark:text-gray-100">Devices</h3>
-
             <ul className="flex text-xs font-medium text-gray-500 dark:text-gray-400 space-x-2">
               { this.renderPill('Size', 'size') }
               { this.renderPill('Browser', 'browser') }
               { this.renderPill('OS', 'os') }
             </ul>
           </div>
-
           { this.renderContent() }
-
         </div>
       </div>
     )