-
Notifications
You must be signed in to change notification settings - Fork 6
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
Add events query/formatter/layout #264
Conversation
…irs/next-build into 15908-events
const formattedDates = formatDateObject(datetimeRange) | ||
|
||
const mostRecentDate = deriveMostRecentDate(formattedDates) | ||
const formattedTimestamp = deriveFormattedTimestamp(mostRecentDate) |
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 wonder if these should be handled inside the query formatter itself. fine here for now, just a thought
src/lib/constants/pageSizes.ts
Outdated
@@ -5,5 +5,6 @@ import { | |||
|
|||
export const PAGE_SIZES = { | |||
[RESOURCE_TYPES.STORY_LISTING]: 10, | |||
[RESOURCE_TYPES.EVENT]: 10, |
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.
this will be for EVENT_LISTING instead of EVENT
const Template: ComponentStory<typeof Event> = (args) => <Event {...args} /> | ||
|
||
export const EventExample = Template.bind({}) | ||
EventExample.args = { |
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.
this feels like it's probably too many args, very likely doesn't need all of this data in order to render (like the nested breadcrumbs on facilityLocation, etc). not a blocker, we can clean it up later... i think most stories need some cleanup
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.
wonderful work. there are a few small follow-up tasks, but those can be handled in a separate ticket & pr. this does a lot to get all of the event-related wiring in place and will be easy to iterate on.
the main follow up tasks i see:
- event breadcrumbs are different from va.gov (home is hidden for us)
- the "see more times" for recurring events shows all immediately, instead of initially trimming to 5
- there appears to be some spacing differences at the bottom, but i like ours better.
- some of the event field types still don't feel DRY to me. we can potentially re-use some things from
paragraph.ts
orfield_types.d.ts
Description
department-of-veterans-affairs/va.gov-cms#15908
Adds Events to next-build.
Changes:
Testing done
Local testing, unit tests
Screenshots
QA steps
Verify that event nodes (ie http://localhost:3000/central-iowa-health-care/events/52265/) render as expected
Verify that events data is properly brought in
Is this PR blocked by another PR?
DO NOT MERGE
label