This repository has been archived by the owner on Jan 9, 2023. It is now read-only.
-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
feat(breadcrumb): add a breadcrumb underneath the page header #1815
Merged
Merged
Changes from 4 commits
Commits
Show all changes
51 commits
Select commit
Hold shift + click to select a range
65b40ae
feat(breadcrumb): add a breadcrumb underneath the page header
oliv37 a68ed7e
feat(breadcrumb): customize breadcrumbs for patients and appointments
oliv37 3e43558
Merge branch 'master' of https://github.com/HospitalRun/hospitalrun-f…
oliv37 63517e8
feat(breadcrumb): add a patient to the store in HispitalRun.tests.tsx
oliv37 a4c1cfb
feat(breadcrumb): use a single component for Breadcrumbs
oliv37 a6eaf2a
feat(breadcrumb): display the breadcrumb in the appointment components
oliv37 7b510cd
Merge branch 'master' into breadcrumbs
150247d
Merge branch 'master' into breadcrumbs
1c5422c
Merge branch 'master' into breadcrumbs
65181bd
Merge branch 'master' into breadcrumbs
01c2537
Merge branch 'master' into breadcrumbs
85c15c2
Merge branch 'master' into breadcrumbs
3642e30
Merge branch 'master' of https://github.com/HospitalRun/hospitalrun-f…
oliv37 8611ad6
Merge branch 'breadcrumbs' of https://github.com/oliv37/hospitalrun-f…
oliv37 8383204
Merge branch 'master' into breadcrumbs
ff6a98f
Merge branch 'master' into breadcrumbs
6910a99
Merge branch 'master' into breadcrumbs
ea4438f
Merge branch 'master' into breadcrumbs
d99bee3
Merge branch 'master' into breadcrumbs
3539ec3
Merge branch 'master' into breadcrumbs
cc91262
Merge branch 'master' into breadcrumbs
8111e2d
Merge branch 'master' into breadcrumbs
764d946
Merge branch 'master' into breadcrumbs
67ad780
Merge branch 'master' into breadcrumbs
87f725d
Merge branch 'master' into breadcrumbs
ef57f20
Merge branch 'master' into breadcrumbs
5beda75
feat(breadcrumb): add hook useAddBreadcrumbs / sort breadcrumbs
oliv37 781b9a5
Merge branch 'breadcrumbs' of https://github.com/oliv37/hospitalrun-f…
oliv37 a44ac9f
feat(breadcrumb): add Breadcrumbs unit tests (component/slice/hook)
oliv37 6d450c9
style(package.json): reset lint-staged formatting
oliv37 dae484a
Merge branch 'master' into breadcrumbs
bbb3ca0
feat(breadcrumb): sort the breadcrumbs in addBreadcrumbs action
oliv37 4dc8a26
Merge branch 'breadcrumbs' of https://github.com/oliv37/hospitalrun-f…
oliv37 492cb37
feat(breadcrumb): use reduxPatient instead of patient for breadcrumbs
oliv37 b5e4c38
feat(breadcrumb): test the dispatch of addBreadcrumbs action
oliv37 1c7a098
Merge branch 'master' into breadcrumbs
94d4fef
Merge branch 'master' into breadcrumbs
f880003
Merge branch 'master' into breadcrumbs
c313e6c
Merge branch 'master' into breadcrumbs
21497c1
Merge branch 'master' into breadcrumbs
24f0c2c
Merge branch 'master' into breadcrumbs
def14db
Merge branch 'master' into breadcrumbs
a845835
Merge branch 'master' into breadcrumbs
d6d6898
Merge branch 'master' into breadcrumbs
05c95ca
Merge branch 'master' into breadcrumbs
e1bfa13
Merge branch 'master' into breadcrumbs
10e902d
Merge branch 'master' into breadcrumbs
beffc1f
feat(breadcrumb): add the dashboard breadcrumb item
oliv37 03174ad
feat(breadcrumb): improve Breadcrumbs tests
oliv37 c71779a
Merge branch 'master' of https://github.com/HospitalRun/hospitalrun-f…
oliv37 478bad0
Merge branch 'master' into breadcrumbs
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
31 changes: 31 additions & 0 deletions
31
src/__tests__/components/breadcrumb/Appointmentbreadcrumb.test.tsx
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,31 @@ | ||
import '../../../__mocks__/matchMediaMock' | ||
import React from 'react' | ||
import { Router } from 'react-router' | ||
import { Provider } from 'react-redux' | ||
import { mount } from 'enzyme' | ||
import configureMockStore from 'redux-mock-store' | ||
import { createMemoryHistory } from 'history' | ||
import { BreadcrumbItem as HrBreadcrumbItem } from '@hospitalrun/components' | ||
import AppointmentBreadcrumb from 'components/breadcrumb/AppointmentBreadcrumb' | ||
|
||
const mockStore = configureMockStore() | ||
|
||
describe('Breadcrumb', () => { | ||
const history = createMemoryHistory() | ||
history.push('/appointments/1234') | ||
const wrapper = mount( | ||
<Provider | ||
store={mockStore({ | ||
appointment: { appointment: {} }, | ||
})} | ||
> | ||
<Router history={history}> | ||
<AppointmentBreadcrumb /> | ||
</Router> | ||
</Provider>, | ||
) | ||
|
||
it('should render 2 breadcrumb items', () => { | ||
expect(wrapper.find(HrBreadcrumbItem)).toHaveLength(2) | ||
}) | ||
}) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,61 @@ | ||
import '../../../__mocks__/matchMediaMock' | ||
import React from 'react' | ||
import { Router } from 'react-router' | ||
import { Provider } from 'react-redux' | ||
import { mount } from 'enzyme' | ||
import configureMockStore from 'redux-mock-store' | ||
import { createMemoryHistory } from 'history' | ||
import DefaultBreadcrumb from 'components/breadcrumb/DefaultBreadcrumb' | ||
import PatientBreadcrumb from 'components/breadcrumb/PatientBreadcrumb' | ||
import AppointmentBreadcrumb from 'components/breadcrumb/AppointmentBreadcrumb' | ||
import Breadcrumb from 'components/breadcrumb/Breadcrumb' | ||
|
||
const mockStore = configureMockStore() | ||
|
||
describe('Breadcrumb', () => { | ||
const setup = (location: string) => { | ||
const history = createMemoryHistory() | ||
history.push(location) | ||
return mount( | ||
<Provider | ||
store={mockStore({ | ||
patient: { patient: {} }, | ||
appointment: { appointment: {} }, | ||
})} | ||
> | ||
<Router history={history}> | ||
<Breadcrumb /> | ||
</Router> | ||
</Provider>, | ||
) | ||
} | ||
it('should render the patient breadcrumb when /patients/:id is accessed', () => { | ||
const wrapper = setup('/patients/1234') | ||
expect(wrapper.find(PatientBreadcrumb)).toHaveLength(1) | ||
}) | ||
it('should render the appointment breadcrumb when /appointments/:id is accessed', () => { | ||
const wrapper = setup('/appointments/1234') | ||
expect(wrapper.find(AppointmentBreadcrumb)).toHaveLength(1) | ||
}) | ||
|
||
it('should render the default breadcrumb when /patients/new is accessed', () => { | ||
const wrapper = setup('/patients/new') | ||
expect(wrapper.find(DefaultBreadcrumb)).toHaveLength(1) | ||
}) | ||
|
||
it('should render the default breadcrumb when /appointments/new is accessed', () => { | ||
const wrapper = setup('/appointments/new') | ||
expect(wrapper.find(DefaultBreadcrumb)).toHaveLength(1) | ||
}) | ||
|
||
it('should render the default breadcrumb when any other path is accessed', () => { | ||
let wrapper = setup('/appointments') | ||
expect(wrapper.find(DefaultBreadcrumb)).toHaveLength(1) | ||
|
||
wrapper = setup('/patients') | ||
expect(wrapper.find(DefaultBreadcrumb)).toHaveLength(1) | ||
|
||
wrapper = setup('/') | ||
expect(wrapper.find(DefaultBreadcrumb)).toHaveLength(1) | ||
}) | ||
}) |
54 changes: 54 additions & 0 deletions
54
src/__tests__/components/breadcrumb/DefaultBreadcrumb.test.tsx
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,54 @@ | ||
import '../../../__mocks__/matchMediaMock' | ||
import React from 'react' | ||
import { mount } from 'enzyme' | ||
import { createMemoryHistory } from 'history' | ||
import { Router } from 'react-router' | ||
import DefaultBreadcrumb, { getItems } from 'components/breadcrumb/DefaultBreadcrumb' | ||
import { BreadcrumbItem as HrBreadcrumbItem } from '@hospitalrun/components' | ||
|
||
describe('DefaultBreadcrumb', () => { | ||
describe('getItems', () => { | ||
it('should return valid items for pathname /', () => { | ||
expect(getItems('/')).toEqual([{ url: '/', active: true }]) | ||
}) | ||
|
||
it('should return valid items for pathname /patients', () => { | ||
expect(getItems('/patients')).toEqual([{ url: '/patients', active: true }]) | ||
}) | ||
|
||
it('should return valid items for pathname /appointments', () => { | ||
expect(getItems('/appointments')).toEqual([{ url: '/appointments', active: true }]) | ||
}) | ||
|
||
it('should return valid items for pathname /patients/new', () => { | ||
expect(getItems('/patients/new')).toEqual([ | ||
{ url: '/patients', active: false }, | ||
{ url: '/patients/new', active: true }, | ||
]) | ||
}) | ||
|
||
it('should return valid items for pathname /appointments/new', () => { | ||
expect(getItems('/appointments/new')).toEqual([ | ||
{ url: '/appointments', active: false }, | ||
{ url: '/appointments/new', active: true }, | ||
]) | ||
}) | ||
}) | ||
|
||
describe('rendering', () => { | ||
const setup = (location: string) => { | ||
const history = createMemoryHistory() | ||
history.push(location) | ||
return mount( | ||
<Router history={history}> | ||
<DefaultBreadcrumb /> | ||
</Router>, | ||
) | ||
} | ||
|
||
it('should render one breadcrumb item for the path /', () => { | ||
const wrapper = setup('/') | ||
expect(wrapper.find(HrBreadcrumbItem)).toHaveLength(1) | ||
}) | ||
}) | ||
}) |
31 changes: 31 additions & 0 deletions
31
src/__tests__/components/breadcrumb/PatientBreadcrumb.test.tsx
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,31 @@ | ||
import '../../../__mocks__/matchMediaMock' | ||
import React from 'react' | ||
import { Router } from 'react-router' | ||
import { Provider } from 'react-redux' | ||
import { mount } from 'enzyme' | ||
import configureMockStore from 'redux-mock-store' | ||
import { createMemoryHistory } from 'history' | ||
import { BreadcrumbItem as HrBreadcrumbItem } from '@hospitalrun/components' | ||
import PatientBreadcrumb from 'components/breadcrumb/PatientBreadcrumb' | ||
|
||
const mockStore = configureMockStore() | ||
|
||
describe('Breadcrumb', () => { | ||
const history = createMemoryHistory() | ||
history.push('/patients/1234') | ||
const wrapper = mount( | ||
<Provider | ||
store={mockStore({ | ||
patient: { patient: {} }, | ||
})} | ||
> | ||
<Router history={history}> | ||
<PatientBreadcrumb /> | ||
</Router> | ||
</Provider>, | ||
) | ||
|
||
it('should render 2 breadcrumb items', () => { | ||
expect(wrapper.find(HrBreadcrumbItem)).toHaveLength(2) | ||
}) | ||
}) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,33 @@ | ||
import React from 'react' | ||
import { useHistory } from 'react-router' | ||
import { useSelector } from 'react-redux' | ||
import { useTranslation } from 'react-i18next' | ||
import { | ||
Breadcrumb as HrBreadcrumb, | ||
BreadcrumbItem as HrBreadcrumbItem, | ||
} from '@hospitalrun/components' | ||
import { RootState } from '../../store' | ||
|
||
const AppointmentBreacrumb = () => { | ||
const { t } = useTranslation() | ||
const { appointment } = useSelector((state: RootState) => state.appointment) | ||
const history = useHistory() | ||
let appointmentLabel = '' | ||
|
||
if (appointment.startDateTime && appointment.endDateTime) { | ||
const startDateLabel = new Date(appointment.startDateTime).toLocaleString() | ||
const endDateLabel = new Date(appointment.endDateTime).toLocaleString() | ||
appointmentLabel = `${startDateLabel} - ${endDateLabel}` | ||
} | ||
|
||
return ( | ||
<HrBreadcrumb> | ||
<HrBreadcrumbItem onClick={() => history.push('/appointments')}> | ||
{t('scheduling.appointments.label')} | ||
</HrBreadcrumbItem> | ||
<HrBreadcrumbItem active>{appointmentLabel}</HrBreadcrumbItem> | ||
</HrBreadcrumb> | ||
) | ||
} | ||
|
||
export default AppointmentBreacrumb |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
import React from 'react' | ||
import { Switch, Route } from 'react-router' | ||
import DefaultBreadcrumb from 'components/breadcrumb/DefaultBreadcrumb' | ||
import PatientBreadcrumb from 'components/breadcrumb/PatientBreadcrumb' | ||
import AppointmentBreadcrumb from 'components/breadcrumb/AppointmentBreadcrumb' | ||
|
||
const Breadcrumb = () => ( | ||
<Switch> | ||
<Route exact path={['/patients/new', '/appointments/new']} component={DefaultBreadcrumb} /> | ||
<Route path="/patients/:id" component={PatientBreadcrumb} /> | ||
<Route path="/appointments/:id" component={AppointmentBreadcrumb} /> | ||
<Route path="*" component={DefaultBreadcrumb} /> | ||
</Switch> | ||
) | ||
|
||
export default Breadcrumb |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,53 @@ | ||
import React from 'react' | ||
import { useLocation, useHistory } from 'react-router' | ||
import { useTranslation } from 'react-i18next' | ||
import { | ||
Breadcrumb as HrBreadcrumb, | ||
BreadcrumbItem as HrBreadcrumbItem, | ||
} from '@hospitalrun/components' | ||
|
||
interface Item { | ||
url: string | ||
active: boolean | ||
} | ||
|
||
const urlToi18nKey: { [url: string]: string } = { | ||
'/': 'dashboard.label', | ||
'/patients': 'patients.label', | ||
'/patients/new': 'patients.newPatient', | ||
'/appointments': 'scheduling.appointments.label', | ||
'/appointments/new': 'scheduling.appointments.new', | ||
} | ||
|
||
export function getItems(pathname: string): Item[] { | ||
let url = '' | ||
const paths = pathname.substring(1).split('/') | ||
|
||
return paths.map((path, index) => { | ||
url += `/${path}` | ||
return { url, active: index === paths.length - 1 } | ||
}) | ||
} | ||
|
||
const DefaultBreadcrumb = () => { | ||
const { t } = useTranslation() | ||
const { pathname } = useLocation() | ||
const history = useHistory() | ||
const items = getItems(pathname) | ||
|
||
return ( | ||
<HrBreadcrumb> | ||
{items.map((item) => { | ||
const onClick = !item.active ? () => history.push(item.url) : undefined | ||
|
||
return ( | ||
<HrBreadcrumbItem key={item.url} active={item.active} onClick={onClick}> | ||
{t(urlToi18nKey[item.url])} | ||
</HrBreadcrumbItem> | ||
) | ||
})} | ||
</HrBreadcrumb> | ||
) | ||
} | ||
|
||
export default DefaultBreadcrumb |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
import React from 'react' | ||
import { useHistory } from 'react-router' | ||
import { useSelector } from 'react-redux' | ||
import { useTranslation } from 'react-i18next' | ||
import { | ||
Breadcrumb as HrBreadcrumb, | ||
BreadcrumbItem as HrBreadcrumbItem, | ||
} from '@hospitalrun/components' | ||
import { getPatientFullName } from 'patients/util/patient-name-util' | ||
import { RootState } from '../../store' | ||
|
||
const PatientBreacrumb = () => { | ||
const { t } = useTranslation() | ||
const { patient } = useSelector((state: RootState) => state.patient) | ||
const history = useHistory() | ||
|
||
return ( | ||
<HrBreadcrumb> | ||
<HrBreadcrumbItem onClick={() => history.push('/patients')}> | ||
{t('patients.label')} | ||
</HrBreadcrumbItem> | ||
<HrBreadcrumbItem active>{getPatientFullName(patient)}</HrBreadcrumbItem> | ||
</HrBreadcrumb> | ||
) | ||
} | ||
|
||
export default PatientBreacrumb |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm wondering if we can combine all of the breadcrumbs into one component.
This breadcrumb component would read from a Provider (defined using the React Context API) or a Redux Slice.
The state of the Provider or Slice would be an array of objects that have two properties: text and location.
The Provider or Slice would have three functions exposed, one to add a breadcrumb, one to remove a breadcrumb, and one to set the breadcrumbs (basically a function to just set the state exactly to what the user passed in).
Each Route Handler (e.g.
Patients
,ViewPatient
,NewPatient
) would then add a breadcrumb to the state, and would remove it during the cleanup phase of theuseEffect
.This would ensure we don't have any duplicate code, the breadcrumb can be a single component, and we don't have any additional work each time a new route or module is added.
Feel free to ping me on slack with any questions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For an example of how this might work, checkout https://github.com/HospitalRun/hospitalrun-frontend/pull/1817/files. I'm working through doing something similar but with a button toolbar.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, if my understanding is correct, each Route Component will declare an
effect
to update the breadcrumb array in the state ?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that you have the correct understanding!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I updated my code using your recommendations (without doing the unit tests).
I only handled the
/
and/patients/**
routes, let met know if it's ok for you, then I will finish the tests and handle the/appointments/**
routesThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just finished to implement the whole feature (I still need to improve the code coverage).
It was not as easy as I expected because of the nested routes. I realized the effect of a child component is executed before the effect of its parent. As a result I couldn't rely on the order of the addBreadcrumb function calls to build a valid breadcrumbs array. So I decided to sort the breadcrumbs by their location property before the rendering.
I am also afraid that maintaining the breadcrumbs will be painful in the future, as the number of routes will increase. Each developers should remember to call the hook
useAddBreadcrumbs
when they add a new route, it can be really cumbersome...