From c95d37583952cbad2e25c2cf9dda9a7b20b155f6 Mon Sep 17 00:00:00 2001 From: Ru Singh Date: Tue, 15 Jun 2021 13:04:43 +0530 Subject: [PATCH] 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 --- .tool-versions | 1 + CONTRIBUTING.md | 21 ++- Makefile | 9 ++ assets/css/app.css | 9 +- assets/js/dashboard/fade-in.js | 12 +- assets/js/dashboard/lazy-loader.js | 12 +- assets/js/dashboard/stats/countries.js | 4 +- assets/js/dashboard/stats/devices/browsers.js | 14 +- assets/js/dashboard/stats/devices/index.js | 129 ++++++++++++------ .../stats/devices/operating-systems.js | 4 +- assets/js/dashboard/stats/more-link.js | 23 +++- .../js/dashboard/stats/pages/entry-pages.js | 23 ++-- assets/js/dashboard/stats/pages/exit-pages.js | 4 +- assets/js/dashboard/stats/pages/index.js | 65 +++++---- assets/js/dashboard/stats/pages/pages.js | 4 +- .../dashboard/stats/sources/referrer-list.js | 31 +++-- .../dashboard/stats/sources/search-terms.js | 6 +- .../js/dashboard/stats/sources/source-list.js | 16 ++- 18 files changed, 261 insertions(+), 126 deletions(-) diff --git a/.tool-versions b/.tool-versions index 675cf7009d1d..12b580032aff 100644 --- 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 \ No newline at end of file diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 1435daf2bbac..9d4d618d46ae 100644 --- 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. \ No newline at end of file +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`. \ No newline at end of file diff --git a/Makefile b/Makefile index 5e7c5c94e4a1..fee3c109696b 100644 --- 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 index 58376b8b2360..e3e0d7660774 100644 --- 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 index 57d8539f7145..6cbe19052d74 100644 --- 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
{children}
+export default function FadeIn({className, show, children}) { + return ( +
+ {children} +
+) } diff --git a/assets/js/dashboard/lazy-loader.js b/assets/js/dashboard/lazy-loader.js index 4a2222fca48c..3824ef943056 100644 --- 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 ( -
this.element = el} className={this.props.className} style={this.props.style}>{this.props.children}
+
{ this.element = el }} + className={this.props.className} + style={this.props.style} + > + {this.props.children} +
); } } diff --git a/assets/js/dashboard/stats/countries.js b/assets/js/dashboard/stats/countries.js index b5f2476865bb..96b02e525370 100644 --- 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 ( -
+
{ this.state.loading &&
} diff --git a/assets/js/dashboard/stats/devices/browsers.js b/assets/js/dashboard/stats/devices/browsers.js index 11150ea3c373..57862dbf7c9d 100644 --- 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 ( - + { this.state.loading &&
} - + { this.renderList() } diff --git a/assets/js/dashboard/stats/devices/index.js b/assets/js/dashboard/stats/devices/index.js index a8d455322350..a07bac3013f5 100644 --- 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,11 +45,6 @@ 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}) @@ -58,11 +52,24 @@ class ScreenSizes extends React.Component { } } + 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) @@ -70,43 +77,59 @@ class ScreenSizes extends React.Component { return (
- - + + {iconFor(size.name)} {size.name}
- {numberFormatter(size.count)} ({size.percentage}%) + + {numberFormatter(size.count)} + ({size.percentage}%) +
) } - label() { - return this.props.query.period === 'realtime' ? 'Current visitors' : 'Visitors' - } - renderList() { if (this.state.sizes && this.state.sizes.length > 0) { return ( -
+
Screen size { this.label() }
{ this.state.sizes && this.state.sizes.map(this.renderScreenSize.bind(this)) } ) - } else { - return
No data yet
} + return ( +
+ No data yet +
+ ) } render() { return ( - + { this.state.loading &&
} - + { this.renderList() } @@ -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 - } else if (this.state.mode === 'browser') { - return - } else if (this.state.mode === 'os') { - return - } - } - + setMode(mode) { return () => { storage.setItem(this.tabKey, mode) this.setState({mode}) } } + + renderContent() { + switch (this.state.mode) { + case 'browser': + return + case 'os': + return ( + + ) + case 'size': + default: + return ( + + ) + } + } renderPill(name, mode) { const isActive = this.state.mode === mode if (isActive) { - return
  • {name}
    • - } else { - return
  • {name}
    • + return ( +
  • + {name} +
    • + ) } + + return ( +
  • + {name} +
    • + ) } render() { return ( -
    -
    - +
    +

    Devices

    -
      { this.renderPill('Size', 'size') } { this.renderPill('Browser', 'browser') } { this.renderPill('OS', 'os') }
    - { this.renderContent() } -
    ) diff --git a/assets/js/dashboard/stats/devices/operating-systems.js b/assets/js/dashboard/stats/devices/operating-systems.js index 7e6011bc5ca3..1b8e1abc0712 100644 --- a/assets/js/dashboard/stats/devices/operating-systems.js +++ b/assets/js/dashboard/stats/devices/operating-systems.js @@ -83,9 +83,9 @@ export default class OperatingSystems extends React.Component { render() { return ( - + { this.state.loading &&
    } - + { this.renderList() }     diff --git a/assets/js/dashboard/stats/more-link.js b/assets/js/dashboard/stats/more-link.js index 6878693bc19c..c0f23cf51d09 100644 --- a/assets/js/dashboard/stats/more-link.js +++ b/assets/js/dashboard/stats/more-link.js @@ -4,9 +4,26 @@ import { Link } from 'react-router-dom' export default function MoreLink({site, list, endpoint}) { if (list.length > 0) { return ( -
    - - +
    + + + {/* eslint-disable-next-line max-len */} + + DETAILS
    diff --git a/assets/js/dashboard/stats/pages/entry-pages.js b/assets/js/dashboard/stats/pages/entry-pages.js index 88c69ea6dc2f..cb6323d770ce 100644 --- a/assets/js/dashboard/stats/pages/entry-pages.js +++ b/assets/js/dashboard/stats/pages/entry-pages.js @@ -42,9 +42,9 @@ export default class EntryPages extends React.Component {
    - + {page.name} - + @@ -57,7 +57,7 @@ export default class EntryPages extends React.Component { renderList() { if (this.state.pages && this.state.pages.length > 0) { return ( - + <>
    Page url Unique Entrances @@ -66,19 +66,24 @@ export default class EntryPages extends React.Component { { this.state.pages.map(this.renderPage.bind(this)) } - + ) - } else { - return
    No data yet
    - } + } + return ( +
    + No data yet +
    + ) } render() { const { loading } = this.state; return ( - + { loading &&
    } - + { this.renderList() } {!loading && } diff --git a/assets/js/dashboard/stats/pages/exit-pages.js b/assets/js/dashboard/stats/pages/exit-pages.js index 397b3142e57a..5397165b6640 100644 --- a/assets/js/dashboard/stats/pages/exit-pages.js +++ b/assets/js/dashboard/stats/pages/exit-pages.js @@ -76,9 +76,9 @@ export default class ExitPages extends React.Component { render() { const { loading } = this.state; return ( - + { loading &&
    } - + { this.renderList() } {!loading && } diff --git a/assets/js/dashboard/stats/pages/index.js b/assets/js/dashboard/stats/pages/index.js index 08a4ab244655..fb6b45ccf084 100644 --- a/assets/js/dashboard/stats/pages/index.js +++ b/assets/js/dashboard/stats/pages/index.js @@ -1,11 +1,9 @@ import React from 'react'; -import { Link } from 'react-router-dom' import * as storage from '../../storage' import Visits from './pages' import EntryPages from './entry-pages' import ExitPages from './exit-pages' -import FadeIn from '../../fade-in' const labelFor = { 'pages': 'Top Pages', @@ -16,23 +14,13 @@ const labelFor = { export default class Pages extends React.Component { constructor(props) { super(props) - this.tabKey = 'pageTab__' + props.site.domain + this.tabKey = `pageTab__${ props.site.domain}` const storedTab = storage.getItem(this.tabKey) this.state = { mode: storedTab || 'pages' } } - renderContent() { - if (this.state.mode === 'pages') { - return - } else if (this.state.mode === 'entry-pages') { - return - } else if (this.state.mode === 'exit-pages') { - return - } - } - setMode(mode) { return () => { storage.setItem(this.tabKey, mode) @@ -40,34 +28,63 @@ export default class Pages extends React.Component { } } + renderContent() { + switch(this.state.mode) { + case "entry-pages": + return + case "exit-pages": + return + case "pages": + default: + return + } + } + + renderPill(name, mode) { const isActive = this.state.mode === mode if (isActive) { - return
  • {name}
    • - } else { - return
  • {name}
    • + return ( +
  • + {name} +
    • + ) } + + return ( +
  • + {name} +
    • + ) } render() { - const filters = this.props.query.filters return ( -
    -
    - +
    +
    + {/* Header Container */}
    -

    {labelFor[this.state.mode] || 'Page Visits'}

    - +

    + {labelFor[this.state.mode] || 'Page Visits'} +

      { this.renderPill('Top Pages', 'pages') } { this.renderPill('Entry Pages', 'entry-pages') } { this.renderPill('Exit Pages', 'exit-pages') }
    - + {/* Main Contents */} { this.renderContent() } -
    ) diff --git a/assets/js/dashboard/stats/pages/pages.js b/assets/js/dashboard/stats/pages/pages.js index 4ad1d7765668..493c3c68fed9 100644 --- a/assets/js/dashboard/stats/pages/pages.js +++ b/assets/js/dashboard/stats/pages/pages.js @@ -87,9 +87,9 @@ export default class Visits extends React.Component { render() { const { loading } = this.state; return ( - + { loading &&
    } - + { this.renderList() } {!loading && } diff --git a/assets/js/dashboard/stats/sources/referrer-list.js b/assets/js/dashboard/stats/sources/referrer-list.js index 071b1967d919..c122fef81f32 100644 --- a/assets/js/dashboard/stats/sources/referrer-list.js +++ b/assets/js/dashboard/stats/sources/referrer-list.js @@ -94,20 +94,27 @@ export default class Referrers extends React.Component { renderList() { if (this.state.referrers.length > 0) { return ( - -
    +
    +
    Referrer { this.label() }
    - + {this.state.referrers.map(this.renderReferrer.bind(this))} - +
    ) - } else { - return
    No data yet
    } + return ( +
    + No data yet +
    + ) } renderContent() { @@ -123,13 +130,15 @@ export default class Referrers extends React.Component { render() { return ( -
    - +
    +

    Top Referrers

    { this.state.loading &&
    } - - { this.renderContent() } - + + { this.renderContent() } +
    ) diff --git a/assets/js/dashboard/stats/sources/search-terms.js b/assets/js/dashboard/stats/sources/search-terms.js index f453126a155d..65018f8cd1ad 100644 --- a/assets/js/dashboard/stats/sources/search-terms.js +++ b/assets/js/dashboard/stats/sources/search-terms.js @@ -107,9 +107,11 @@ export default class SearchTerms extends React.Component { render() { return ( -
    +
    { this.state.loading &&
    } - + { this.renderContent() }
    diff --git a/assets/js/dashboard/stats/sources/source-list.js b/assets/js/dashboard/stats/sources/source-list.js index 1bd378d5084c..40ca9003a8bb 100644 --- a/assets/js/dashboard/stats/sources/source-list.js +++ b/assets/js/dashboard/stats/sources/source-list.js @@ -99,7 +99,9 @@ class AllSources extends React.Component { render() { return ( -
    +
    { this.renderContent() }
    ) @@ -166,17 +168,17 @@ class UTMSources extends React.Component { renderList() { if (this.state.referrers && this.state.referrers.length > 0) { return ( - +
    {UTM_TAGS[this.props.tab].label} {this.label()}
    - + {this.state.referrers.map(this.renderReferrer.bind(this))} - +
    ) } else { return
    No data yet
    @@ -191,7 +193,7 @@ class UTMSources extends React.Component { { this.props.renderTabs() }
    { this.state.loading &&
    } - + { this.renderList() } @@ -200,7 +202,9 @@ class UTMSources extends React.Component { render() { return ( -
    +
    { this.renderContent() }
    )