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

fix: make space to left of dashboard items scrollable [DHIS2-10138] #1523

Merged
merged 7 commits into from
Feb 12, 2021
36 changes: 0 additions & 36 deletions cypress/integration/ui/view_dashboard/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,42 +21,6 @@ When('I press enter in the search dashboard field', () => {
cy.get('[data-test="search-dashboard-input"]').type('{enter}')
})

When('I click to preview the print layout', () => {
cy.get('button').contains('More').click()
cy.get('[data-test="print-menu-item"]').click()
cy.get('[data-test="print-layout-menu-item"]').click()
})

Then('the print layout displays for {string} dashboard', title => {
//check the url
cy.location().should(loc => {
expect(loc.hash).to.equal(`${dashboards[title].route}/printlayout`)
})

//check for some elements
cy.get('[data-test="print-layout-page"]').should('be.visible')
})

When('I click to exit print preview', () => {
cy.contains('Exit print preview').click()
})

When('I click to preview the print one-item-per-page', () => {
cy.get('button').contains('More').click()
cy.get('[data-test="print-menu-item"]').click()
cy.get('[data-test="print-oipp-menu-item"]').click()
})

Then('the print one-item-per-page displays for {string} dashboard', title => {
//check the url
cy.location().should(loc => {
expect(loc.hash).to.equal(`${dashboards[title].route}/printoipp`)
})

//check for some elements
cy.get('[data-test="print-oipp-page"]').should('be.visible')
})

When('I search for dashboards containing Noexist', () => {
cy.get('[data-test="search-dashboard-input"]').type('Noexist')
})
Expand Down
38 changes: 38 additions & 0 deletions cypress/integration/ui/view_dashboard/print.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
import { When, Then } from 'cypress-cucumber-preprocessor/steps'
import { dashboards } from '../../../assets/backends/sierraLeone_236'

When('I click to preview the print layout', () => {
cy.get('button').contains('More').click()
cy.get('[data-test="print-menu-item"]').click()
cy.get('[data-test="print-layout-menu-item"]').click()
})

Then('the print layout displays for {string} dashboard', title => {
//check the url
cy.location().should(loc => {
expect(loc.hash).to.equal(`${dashboards[title].route}/printlayout`)
})

//check for some elements
cy.get('[data-test="print-layout-page"]').should('be.visible')
})

When('I click to exit print preview', () => {
cy.get('button').not('.small').contains('Exit print preview').click()
})

When('I click to preview the print one-item-per-page', () => {
cy.get('button').contains('More').click()
cy.get('[data-test="print-menu-item"]').click()
cy.get('[data-test="print-oipp-menu-item"]').click()
})

Then('the print one-item-per-page displays for {string} dashboard', title => {
//check the url
cy.location().should(loc => {
expect(loc.hash).to.equal(`${dashboards[title].route}/printoipp`)
})

//check for some elements
cy.get('[data-test="print-oipp-page"]').should('be.visible')
})
37 changes: 24 additions & 13 deletions src/components/Dashboard/PrintActionsBar.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@ import i18n from '@dhis2/d2-i18n'
import { Button } from '@dhis2/ui'
import { Link } from 'react-router-dom'
import LessHorizontalIcon from '../../icons/LessHorizontal'
import { useWindowDimensions } from '../WindowDimensionsProvider'
import { isSmallScreen } from '../../modules/smallScreen'

import classes from './styles/PrintActionsBar.module.css'

Expand All @@ -14,25 +12,38 @@ export const PRINT_ACTIONS_BAR_HEIGHT = 44
export const PRINT_ACTIONS_BAR_HEIGHT_SM = 36

const PrintActionsBar = ({ id }) => {
const { width } = useWindowDimensions()
const isSmall = isSmallScreen(width)
const getExitPrintButton = isSmall => (
<Button
className={isSmall ? classes.buttonSmall : classes.buttonLarge}
small={isSmall}
>
<LessHorizontalIcon />
{i18n.t('Exit print preview')}
</Button>
)

const getPrintButton = isSmall => (
<Button
className={isSmall ? classes.buttonSmall : classes.buttonLarge}
small={isSmall}
onClick={window.print}
>
{i18n.t('Print')}
</Button>
)

return (
<>
<div className={classes.container}>
<div className={classes.inner}>
<div className={classes.actions}>
<Link className={classes.link} to={`/${id}`}>
<Button small={isSmall}>
<LessHorizontalIcon />
{i18n.t('Exit print preview')}
</Button>
{getExitPrintButton(true)}
{getExitPrintButton(false)}
</Link>
<Button small={isSmall} onClick={window.print}>
{i18n.t('Print')}
</Button>
{getPrintButton(true)}
{getPrintButton(false)}
</div>
</div>
<div className={classes.topMargin} />
</>
)
}
Expand Down
2 changes: 1 addition & 1 deletion src/components/Dashboard/PrintDashboard.js
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ const PrintDashboard = ({
<>
<PrintActionsBar id={dashboard.id} />
<div
className={classes.wrapper}
className={classes.container}
style={{ height: availableHeight }}
>
<PrintInfo isLayout={false} />
Expand Down
6 changes: 3 additions & 3 deletions src/components/Dashboard/PrintLayoutDashboard.js
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ const PrintLayoutDashboard = ({
}
}, [dashboard, items])

const getWrapperStyle = () => {
const getContainerStyle = () => {
return fromEdit
? {
paddingTop: spacers.dp24,
Expand All @@ -103,8 +103,8 @@ const PrintLayoutDashboard = ({
<>
{!fromEdit && <PrintActionsBar id={dashboard.id} />}
<div
className={cx(classes.wrapper, 'scroll-area')}
style={getWrapperStyle()}
className={cx(classes.container, 'scroll-area')}
style={getContainerStyle()}
>
{!fromEdit && <PrintInfo isLayout={true} />}
<div
Expand Down
25 changes: 12 additions & 13 deletions src/components/Dashboard/styles/PrintActionsBar.module.css
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,10 @@
background-color: var(--colors-grey600);
width: 100%;
padding: var(--spacers-dp4) 0;
position: fixed;
z-index: 10;
}

.topMargin {
margin-top: 44px; /* PRINT_ACTIONS_BAR_HEIGHT */
}

.inner {
.actions {
margin-left: 44px;
margin-right: 30px;
display: flex;
Expand All @@ -25,26 +20,30 @@
margin-right: var(--spacers-dp4);
}

.actions .buttonSmall {
display: none;
}

@media only screen and (max-width: 480px) {
.container {
padding-bottom: 3px;
}

.inner {
.actions {
margin: 0 var(--spacers-dp8);
}

.topMargin {
margin-top: 36px; /* PRINT_ACTIONS_BAR_HEIGHT_SM */
.actions .buttonSmall {
display: inline-flex;
}

.actions .buttonLarge {
display: none;
}
}

@media print {
.container {
display: none;
}

.topMargin {
margin-top: 0px;
}
}
14 changes: 9 additions & 5 deletions src/components/Dashboard/styles/PrintDashboard.module.css
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
.wrapper {
.container {
background-color: #f4f6f8;
margin-left: var(--spacers-dp32);
padding-left: var(--spacers-dp12);
padding-left: 44px;
overflow-y: auto;
}

Expand All @@ -14,12 +13,17 @@
width: 1102px;
}

@media only screen and (max-width: 480px) {
.container {
padding-left: var(--spacers-dp12);
}
}

@media print {
.wrapper {
.container {
background-color: white;
height: auto !important;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably out of scope, but I noticed these !important. media -> class should override any class selector, so are these really necessary?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've been starting to review a lot of the css and I'll make a note to look at this one. Thanks!

overflow: visible !important;
margin-left: 0px;
padding-left: 0px;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also not in scope, and it has no production code impact, but my personal preference is to use 0 instead of 0px. This is just my OCD speaking, so disregard this if you have a different opinion 😄

}

Expand Down
20 changes: 9 additions & 11 deletions src/components/Dashboard/styles/PrintLayoutDashboard.module.css
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
.wrapper {
.container {
background-color: #f4f6f8;
jenniferarnesen marked this conversation as resolved.
Show resolved Hide resolved
margin-left: var(--spacers-dp32);
padding-left: var(--spacers-dp12);
padding-left: 44px;
overflow-y: auto;
}

Expand All @@ -14,12 +13,17 @@
width: 1102px;
}

@media only screen and (max-width: 480px) {
.container {
padding-left: var(--spacers-dp12);
}
}

@media print {
.wrapper {
.container {
background-color: white;
height: auto !important;
overflow: visible !important;
margin-left: 0px;
padding-left: 0px;
}

Expand All @@ -28,9 +32,3 @@
border-radius: 0px;
}
}

@media only screen and (max-width: 480px) {
.wrapper {
margin-left: 0px;
}
}