-
Notifications
You must be signed in to change notification settings - Fork 2
Multiday events #197
Multiday events #197
Changes from 4 commits
9395e3c
5022309
13f996d
7f2efc4
a7edbfb
845a49b
125733e
c4c2dcd
6854a62
4921dd8
79d780d
7351d54
7fe089d
69ef957
fa49964
c0b5025
74a69ad
8b2c1c3
27639a0
3f61db6
3897ca8
550b6b4
1e95e3e
1183c19
71d2841
3f863d2
985e661
2ab654e
f543838
99702eb
aa22624
a65d40b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,45 @@ | ||
import React, { PropTypes } from 'react'; | ||
import styles from './style.css'; | ||
|
||
import TagsList from '../tags-list'; | ||
import EventLinksList from '../event-links-list'; | ||
|
||
const EventMeta = ({ | ||
event, | ||
}) => ( | ||
<div> | ||
{ | ||
event.externalLinks || event.internalLinks ? | ||
<div className={styles.eventLinks}> | ||
{ | ||
event.externalLinks ? | ||
<EventLinksList | ||
linkList={event.externalLinks} | ||
listType="external" /> | ||
: null | ||
} | ||
{ | ||
event.internalLinks ? | ||
<EventLinksList | ||
linkList={event.internalLinks} | ||
listType="internal" /> | ||
: null | ||
} | ||
</div> | ||
: null | ||
} | ||
{ | ||
event.tags | ||
? <TagsList | ||
tags={event.tags} | ||
tagsLinkPath="about-us/events" /> | ||
: null | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note that we need to update to React 15 in order to get that |
||
} | ||
</div> | ||
); | ||
|
||
EventMeta.propTypes = { | ||
event: PropTypes.object.isRequired, | ||
}; | ||
|
||
export default EventMeta; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
.eventLinks { | ||
composes: cf from "../../components/utils/layout.css"; | ||
margin-bottom: 1em; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,62 @@ | ||
import React, { PropTypes } from 'react'; | ||
import styles from './style.css'; | ||
import EventImage from '../event-image'; | ||
import DateBubble from '../date-bubble'; | ||
import EventMeta from '../event-meta'; | ||
import EventTitle from '../event-title'; | ||
import HR from '../hr'; | ||
import { Grid, Cell } from '../grid'; | ||
import { setEndDate, eventImagePath } from '../../util/events'; | ||
import { eventHref } from '../../util/event'; | ||
|
||
const EventsListEntry = ({ | ||
event, | ||
timeline, | ||
}) => ( | ||
<li key={`event_${event.id}`} className={styles.eventItem}> | ||
<Grid fit={false}> | ||
<Cell size={12}> | ||
<HR color="grey" customClassName= | ||
{styles.mobileHorizontalLine} /> | ||
<DateBubble | ||
startDateTime={event.startDateTime} | ||
endDateTime={setEndDate( | ||
timeline, | ||
event.startDateTime, | ||
event.endDateTime)} | ||
/> | ||
</Cell> | ||
<Cell size={1} key="event_picture_mobile" hideOn="mobileSM"> | ||
<EventImage | ||
imgPath={eventImagePath(event.featureImageFilename)} | ||
href={eventHref(event)} /> | ||
</Cell> | ||
<Cell size={12} breakOn="mobile"> | ||
<Grid fit={false}> | ||
<Cell size={8} key='event_description' | ||
breakOn="mobileS"> | ||
<EventTitle eventTitle={event.title} | ||
eventHref={eventHref(event)} /> | ||
<div className={styles.eventDescription}> | ||
{event.strapline} | ||
</div> | ||
<EventMeta event={event} /> | ||
</Cell> | ||
<Cell size={4} key='event_picture' breakOn="mobileS" | ||
hideOn="mobileS"> | ||
<EventImage | ||
imgPath={eventImagePath(event.featureImageFilename)} | ||
href={eventHref(event)} /> | ||
</Cell> | ||
</Grid> | ||
</Cell> | ||
</Grid> | ||
</li> | ||
); | ||
|
||
EventsListEntry.propTypes = { | ||
event: PropTypes.object.isRequired, | ||
timeline: PropTypes.oneOf(['past', 'future', 'today']).isRequired, | ||
}; | ||
|
||
export default EventsListEntry; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
.eventItem { | ||
position: relative; | ||
list-style-type: none; | ||
} | ||
|
||
.mobileHorizontalLine { | ||
display: block; | ||
margin: 1.5em 0 0 0; | ||
} | ||
|
||
.eventDescription { | ||
margin-top: 10px; | ||
float: left; | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,18 +4,9 @@ | |
|
||
import React, { PropTypes } from 'react'; | ||
import styles from './style.css'; | ||
|
||
import EventImage from '../event-image'; | ||
import DateBubble from '../date-bubble'; | ||
import HR from '../hr'; | ||
import { Grid, Cell } from '../grid'; | ||
|
||
import TagsList from '../tags-list'; | ||
import EventLinksList from '../event-links-list'; | ||
import EventsTimelineTitle from '../events-timeline-title'; | ||
import EventTitle from '../event-title'; | ||
import { eventHref } from '../../util/event'; | ||
import { splitEvents, setEndDate, eventImagePath } from '../../util/events'; | ||
import EventsListEntry from '../events-list-entry'; | ||
import { splitEvents } from '../../util/events'; | ||
|
||
const EventsList = ({ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Lots of new logic in this component, but no new tests. Do the existing ones cover the new paths? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Turned out this component has no tests at all. Adding tests. |
||
events, | ||
|
@@ -37,79 +28,19 @@ const EventsList = ({ | |
<ul className={styles.eventsList}> | ||
{ | ||
relevantEvents.map((event) => ( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is quite a complex component. Could simplify it further here to have a child '<RelevantEvent ... />' etc. I think more complex tests are needed to ensure that all the ? checks achieve the desired result - seems easy to break |
||
<li key={`event_${event.id}`} className={styles.eventItem}> | ||
<Grid fit={false}> | ||
<Cell size={12}> | ||
<HR color="grey" customClassName= | ||
{styles.mobileHorizontalLine} /> | ||
<DateBubble | ||
startDateTime={event.startDateTime} | ||
endDateTime={setEndDate( | ||
timeline, | ||
event.startDateTime, | ||
event.endDateTime)} | ||
/> | ||
</Cell> | ||
<Cell size={1} key="event_picture_mobile" hideOn="mobileSM"> | ||
<EventImage | ||
imgPath={eventImagePath(event.featureImageFilename)} | ||
href={eventHref(event)} /> | ||
</Cell> | ||
<Cell size={12} breakOn="mobile"> | ||
<Grid fit={false}> | ||
<Cell size={8} key='event_description' | ||
breakOn="mobileS"> | ||
<EventTitle eventTitle={event.title} | ||
eventHref={eventHref(event)} /> | ||
<div className={styles.eventDescription}> | ||
{event.strapline} | ||
</div> | ||
{ | ||
event.externalLinks || event.internalLinks ? | ||
<div className={styles.eventLinks}> | ||
{ | ||
event.externalLinks ? | ||
<EventLinksList | ||
linkList={event.externalLinks} | ||
listType="external" /> | ||
: null | ||
} | ||
{ | ||
event.internalLinks ? | ||
<EventLinksList | ||
linkList={event.internalLinks} | ||
listType="internal" /> | ||
: null | ||
} | ||
</div> | ||
: null | ||
} | ||
{ | ||
event.tags | ||
? <TagsList | ||
tags={event.tags} | ||
tagsLinkPath="about-us/events" /> | ||
: null | ||
} | ||
</Cell> | ||
<Cell size={4} key='event_picture' breakOn="mobileS" | ||
hideOn="mobileS"> | ||
<EventImage | ||
imgPath={eventImagePath(event.featureImageFilename)} | ||
href={eventHref(event)} /> | ||
</Cell> | ||
</Grid> | ||
</Cell> | ||
</Grid> | ||
</li> | ||
<EventsListEntry | ||
event={event} | ||
timeline={timeline} | ||
key={`key_${event.id}`} | ||
/> | ||
)) | ||
} | ||
</ul> | ||
</div> | ||
); | ||
} | ||
|
||
return null; | ||
return (<noscript />); | ||
}; | ||
|
||
EventsList.propTypes = { | ||
|
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.
Would it make more sense to do this inside the child component?
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.
Yeah probably. There's also potential for refactoring in a way of only passing down elements of the
event
and not the wholeevent
.