Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: keep dashboard title visible even when user is scrolling [v39] #3156

Draft
wants to merge 12 commits into
base: v39
Choose a base branch
from
19 changes: 11 additions & 8 deletions i18n/en.pot
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,15 @@ msgstr ""
"Content-Type: text/plain; charset=utf-8\n"
"Content-Transfer-Encoding: 8bit\n"
"Plural-Forms: nplurals=2; plural=(n != 1)\n"
"POT-Creation-Date: 2023-05-10T12:25:45.620Z\n"
"PO-Revision-Date: 2023-05-10T12:25:45.620Z\n"
"POT-Creation-Date: 2025-01-06T12:08:21.892Z\n"
"PO-Revision-Date: 2025-01-06T12:08:21.908Z\n"

msgid "Untitled dashboard"
msgstr "Untitled dashboard"

msgid "No dashboards found for \"{{filterText}}\""
msgstr "No dashboards found for \"{{filterText}}\""

msgid "Cannot create a dashboard while offline"
msgstr "Cannot create a dashboard while offline"

Expand Down Expand Up @@ -478,12 +481,6 @@ msgstr "Yes, remove filters"
msgid "The dashboard couldn't be made available offline. Try again."
msgstr "The dashboard couldn't be made available offline. Try again."

msgid "Failed to unstar the dashboard"
msgstr "Failed to unstar the dashboard"

msgid "Failed to star the dashboard"
msgstr "Failed to star the dashboard"

msgid "Remove from offline storage"
msgstr "Remove from offline storage"

Expand Down Expand Up @@ -548,6 +545,12 @@ msgstr "Cannot unstar this dashboard while offline"
msgid "Cannot star this dashboard while offline"
msgstr "Cannot star this dashboard while offline"

msgid "Failed to unstar the dashboard"
msgstr "Failed to unstar the dashboard"

msgid "Failed to star the dashboard"
msgstr "Failed to star the dashboard"

msgid "Loading dashboard – {{name}}"
msgstr "Loading dashboard – {{name}}"

Expand Down
2 changes: 1 addition & 1 deletion src/components/App.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,12 +43,12 @@
'--headerbar-height',
`${headerbarHeight}px`
)
}, [])

Check warning on line 46 in src/components/App.js

View workflow job for this annotation

GitHub Actions / lint

React Hook useEffect has a missing dependency: 'props'. Either include it or remove the dependency array. However, 'props' will change when *any* prop changes, so the preferred fix is to destructure the 'props' object outside of the useEffect call and refer to those specific props inside useEffect

return (
systemSettings && (
<>
<CssVariables colors spacers />
<CssVariables colors spacers elevations />
<Router>
<Switch>
<Route
Expand Down
4 changes: 4 additions & 0 deletions src/components/DashboardsBar/Chip.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@ const Chip = ({ starred, selected, label, dashboardId, onClick }) => {
const { isConnected: online } = useDhis2ConnectionStatus()
const chipProps = {
selected,
marginLeft: 0,
marginRight: 0,
marginTop: 0,
marginBottom: 0,
}

if (starred) {
Expand Down
23 changes: 21 additions & 2 deletions src/components/DashboardsBar/Content.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { sGetSelectedId } from '../../reducers/selected.js'
import Chip from './Chip.js'
import Filter from './Filter.js'
import { getFilteredDashboards } from './getFilteredDashboards.js'
import ShowMoreButton from './ShowMoreButton.js'
import classes from './styles/Content.module.css'

const Content = ({
Expand Down Expand Up @@ -39,8 +40,19 @@ const Content = ({
}
}

const getChips = () =>
getFilteredDashboards(dashboards, filterText).map((dashboard) => (
const getChips = () => {
const filteredList = getFilteredDashboards(dashboards, filterText)

if (filteredList.length === 0) {
return (
<span className={classes.noDashboardsMessage}>
{i18n.t('No dashboards found for "{{filterText}}"', {
filterText,
})}
</span>
)
}
return filteredList.map((dashboard) => (
<Chip
key={dashboard.id}
label={dashboard.displayName}
Expand All @@ -50,6 +62,7 @@ const Content = ({
onClick={onChipClicked}
/>
))
}

const getControlsSmall = () => (
<div className={classes.controlsSmall}>
Expand All @@ -58,6 +71,12 @@ const Content = ({
onSearchClicked={onSearchClicked}
expanded={expanded}
/>
{expanded && (
<ShowMoreButton
onClick={onSearchClicked}
dashboardBarIsExpanded={expanded}
/>
)}
</div>
)

Expand Down
13 changes: 7 additions & 6 deletions src/components/DashboardsBar/DashboardsBar.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,23 +53,23 @@
updateUserRows(Math.min(newRows, MAX_ROW_COUNT))
userRowsChanged.current = true
}
}, [mouseYPos])

Check warning on line 56 in src/components/DashboardsBar/DashboardsBar.js

View workflow job for this annotation

GitHub Actions / lint

React Hook useEffect has missing dependencies: 'onExpandedChanged', 'updateUserRows', and 'userRows'. Either include them or remove the dependency array. If 'onExpandedChanged' changes too often, find the parent component that defines it and wrap that definition in useCallback

useEffect(() => {
rootElement.style.setProperty('--user-rows-count', userRows)
}, [userRows])

Check warning on line 60 in src/components/DashboardsBar/DashboardsBar.js

View workflow job for this annotation

GitHub Actions / lint

React Hook useEffect has a missing dependency: 'rootElement.style'. Either include it or remove the dependency array

useEffect(() => {
const vh = height * 0.01
rootElement.style.setProperty('--vh', `${vh}px`)
}, [height])

Check warning on line 65 in src/components/DashboardsBar/DashboardsBar.js

View workflow job for this annotation

GitHub Actions / lint

React Hook useEffect has a missing dependency: 'rootElement.style'. Either include it or remove the dependency array

useEffect(() => {
if (!dragging && userRowsChanged.current) {
apiPostControlBarRows(userRows)
userRowsChanged.current = false
}
}, [dragging, userRowsChanged.current])

Check warning on line 72 in src/components/DashboardsBar/DashboardsBar.js

View workflow job for this annotation

GitHub Actions / lint

React Hook useEffect has a missing dependency: 'userRows'. Either include it or remove the dependency array. Mutable values like 'userRowsChanged.current' aren't valid dependencies because mutating them doesn't re-render the component

const scrollToTop = () => {
if (expanded) {
Expand All @@ -84,12 +84,12 @@
scrollToTop()
onExpandedChanged(!expanded)
}
}, [expanded])

Check warning on line 87 in src/components/DashboardsBar/DashboardsBar.js

View workflow job for this annotation

GitHub Actions / lint

React Hook useCallback has missing dependencies: 'memoizedCancelExpanded', 'onExpandedChanged', and 'scrollToTop'. Either include them or remove the dependency array. If 'onExpandedChanged' changes too often, find the parent component that defines it and wrap that definition in useCallback

const memoizedCancelExpanded = useCallback(() => {
scrollToTop()
onExpandedChanged(false)
}, [])

Check warning on line 92 in src/components/DashboardsBar/DashboardsBar.js

View workflow job for this annotation

GitHub Actions / lint

React Hook useCallback has missing dependencies: 'onExpandedChanged' and 'scrollToTop'. Either include them or remove the dependency array. If 'onExpandedChanged' changes too often, find the parent component that defines it and wrap that definition in useCallback

return (
<div
Expand All @@ -110,17 +110,18 @@
expanded={expanded}
/>
</div>
<ShowMoreButton
onClick={memoizedToggleExpanded}
dashboardBarIsExpanded={expanded}
disabled={!expanded && userRows === MAX_ROW_COUNT}
/>
{!expanded && (
<ShowMoreButton
onClick={memoizedToggleExpanded}
dashboardBarIsExpanded={expanded}
disabled={!expanded && userRows === MAX_ROW_COUNT}
/>
)}
<DragHandle
setDragging={setDragging}
onHeightChanged={setMouseYPos}
/>
</div>
<div className={classes.spacer} />
</div>
)
}
Expand Down
9 changes: 4 additions & 5 deletions src/components/DashboardsBar/ShowMoreButton.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
import i18n from '@dhis2/d2-i18n'
import { Tooltip } from '@dhis2/ui'
import { IconChevronDown24, IconChevronUp24, Tooltip } from '@dhis2/ui'
import PropTypes from 'prop-types'
import React, { useRef } from 'react'
import { ChevronDown, ChevronUp } from './assets/icons.js'
import classes from './styles/ShowMoreButton.module.css'

const ShowMoreButton = ({ onClick, dashboardBarIsExpanded, disabled }) => {
Expand Down Expand Up @@ -31,7 +30,7 @@ const ShowMoreButton = ({ onClick, dashboardBarIsExpanded, disabled }) => {
<div className={classes.container} ref={containerRef}>
{disabled ? (
<div className={classes.disabled}>
<ChevronDown />
<IconChevronDown24 />
</div>
) : (
<Tooltip
Expand All @@ -51,9 +50,9 @@ const ShowMoreButton = ({ onClick, dashboardBarIsExpanded, disabled }) => {
onMouseOut={onMouseOut}
>
{dashboardBarIsExpanded ? (
<ChevronUp />
<IconChevronUp24 />
) : (
<ChevronDown />
<IconChevronDown24 />
)}
</button>
)}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ exports[`ShowMoreButton renders correctly when at maxHeight 1`] = `
xmlns="http://www.w3.org/2000/svg"
>
<path
d="m11.29289 15.7071c.39053.3905 1.02369.3905 1.41422 0l4.99999-4.99999c.3905-.39053.3905-1.02369 0-1.41422-.3905-.39052-1.0237-.39052-1.4142 0l-4.2929 4.2929-4.29289-4.2929c-.39053-.39052-1.02369-.39052-1.41422 0-.39052.39053-.39052 1.02369 0 1.41422z"
fill="#a0adba"
d="M11.293 15.707a1 1 0 001.414 0l5-5a1 1 0 00-1.414-1.414L12 13.586 7.707 9.293a1 1 0 00-1.414 1.414z"
fill="currentColor"
/>
</svg>
</button>
Expand All @@ -43,8 +43,8 @@ exports[`ShowMoreButton renders correctly when not at maxHeight 1`] = `
xmlns="http://www.w3.org/2000/svg"
>
<path
d="m11.29289 15.7071c.39053.3905 1.02369.3905 1.41422 0l4.99999-4.99999c.3905-.39053.3905-1.02369 0-1.41422-.3905-.39052-1.0237-.39052-1.4142 0l-4.2929 4.2929-4.29289-4.2929c-.39053-.39052-1.02369-.39052-1.41422 0-.39052.39053-.39052 1.02369 0 1.41422z"
fill="#a0adba"
d="M11.293 15.707a1 1 0 001.414 0l5-5a1 1 0 00-1.414-1.414L12 13.586 7.707 9.293a1 1 0 00-1.414 1.414z"
fill="currentColor"
/>
</svg>
</button>
Expand Down
6 changes: 0 additions & 6 deletions src/components/DashboardsBar/__tests__/getRowsFromHeight.js

This file was deleted.

27 changes: 27 additions & 0 deletions src/components/DashboardsBar/__tests__/getRowsFromHeight.spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
import { getRowsFromHeight } from '../getRowsFromHeight.js'

test('getRowsFromHeight returns an integer', () => {
const res = getRowsFromHeight(100)
expect(Number.isInteger(res)).toBeTruthy()
})

const testCases = [
{ height: 0, rows: 1 },
{ height: 36, rows: 1 },
{ height: 62, rows: 1 },
{ height: 63, rows: 2 },
{ height: 100, rows: 2 },
{ height: 101, rows: 3 },
{ height: 138, rows: 3 },
{ height: 139, rows: 4 },
{ height: 200, rows: 5 },
{ height: 300, rows: 8 },
{ height: 400, rows: 10 },
{ height: 500, rows: 13 },
]

testCases.forEach(({ height, rows }) => {
test(`getRowsFromHeight returns ${rows} for height ${height}`, () => {
expect(getRowsFromHeight(height)).toBe(rows)
})
})
16 changes: 10 additions & 6 deletions src/components/DashboardsBar/getRowsFromHeight.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
const ROW_HEIGHT = 40
const PADDING_TOP = 10
const SHOWMORE_BUTTON_HEIGHT = 21 // 27px - 6px below bottom edge of ctrlbar
/*
ROW_HEIGHT is the height of a chip + the gap between chips.
An extra 6px for the top padding is applied to the FIRST_ROW_HEIGHT
*/
const FIRST_ROW_HEIGHT = 44 // 32px chip + 6x top padding + 6px gap
const ROW_HEIGHT = 38 // 32px chip + 6px gap

export const getRowsFromHeight = (height) => {
return Math.round(
(height - SHOWMORE_BUTTON_HEIGHT - PADDING_TOP) / ROW_HEIGHT
)
if (height <= FIRST_ROW_HEIGHT) {
return 1
}
return 1 + Math.abs(Math.round((height - FIRST_ROW_HEIGHT) / ROW_HEIGHT))
}
6 changes: 0 additions & 6 deletions src/components/DashboardsBar/styles/Chip.module.css
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,3 @@
.progressIndicator.selected {
color: var(--colors-white);
}

@media only screen and (max-width: 480px) {
.link {
margin: 0 -2px;
}
}
39 changes: 22 additions & 17 deletions src/components/DashboardsBar/styles/Content.module.css
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,10 @@
.controlsLarge {
display: inline-flex;
position: relative;
top: 5px;
}

.buttonPadding {
padding: 2px var(--spacers-dp8) 0 var(--spacers-dp8);
padding: 0 var(--spacers-dp8) 0 0;
display: inline-flex;
}

Expand All @@ -23,17 +22,31 @@
}

.chipsContainer {
min-height: 40px;
min-height: 32px;
display: flex;
flex-wrap: wrap;
gap: 6px;
}

.noDashboardsMessage {
padding-left: 6px;
color: #4A5768;
font-size: 12px;
display: inline-flex;
-webkit-box-align: center;
align-items: center;
height: 32px;
line-height: 16px;
user-select: none;
}
@media only screen and (max-width: 480px) {
.newLink {
display: none;
}

.controlsSmall {
display: block;
margin-bottom: var(--spacers-dp4);
display: flex;
justify-content: space-between;
}

.controlsLarge {
Expand All @@ -42,17 +55,15 @@

.container.collapsed {
display: flex;
overflow-x: auto;
overflow-y: hidden;
padding: var(--spacers-dp4) 0 var(--spacers-dp4) var(--spacers-dp4);
overflow: hidden;
}

.container.expanded {
display: flex;
flex-direction: column;
overflow: hidden;
padding-top: var(--spacers-dp12);
padding-left: var(--spacers-dp8);
padding-top: var(--spacers-dp6);
padding-left: var(--spacers-dp6);
}

.expanded .chipsContainer .controls {
Expand All @@ -61,12 +72,6 @@
width: 100%;
}

.chipsContainer {
margin-bottom: -4px;
padding-right: 2px;
min-height: 0;
}

.expanded .chipsContainer {
overflow-x: hidden;
overflow-y: auto;
Expand All @@ -75,9 +80,9 @@
}

.collapsed .chipsContainer {
overflow-x: auto;
overflow-y: hidden;
display: flex;
flex-wrap: nowrap;
padding-bottom: 6px;
}
}
Loading
Loading