-
Notifications
You must be signed in to change notification settings - Fork 8
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
DDFFORM 417 #1076
Merged
Merged
DDFFORM 417 #1076
Conversation
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
kasperg
requested changes
Apr 4, 2024
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.
👍 Nice changes here. I like your hook implementation.
I think some things here need a second look. This includes handling of end user texts.
This is the main component and story for the OpeningHours app that displays the opening hours for branches. Depends on several pother commits for functionality. DDFFORM-417
This adds the components for display a week of opening hour entries. Depends on several other commits for functionality. DDFFORM-417
DDFFORM-417
This controls the logic for organizing the query'ed data and navigating the weeks being displayed. DDFFORM-417
These are all the helpers used for the openingHours app. DDFFORM-417
DDFFORM-417
This adds a wiremock for the CMS which can be used to replicate the API structure for testing. DDFFORM-417
Consider if this data is the data / endpoints we want to go forward for mocked data. DDFFORM-417
I am unsure, if this should be commit here, or added in in another fashion. It is generated on the target: https://raw.githubusercontent.com/danskernesdigitalebibliotek/dpl-cms/opening-hours-api/openapi.json Includes not only the necessary GET endpoint etc for this ticket, but also other CRUD. DDFFORM-417
In a previous commit related to OpeningHours, a global locale is set to danish. This change made this test fail as the expected string is hardcoded to english. For now the string is manually changed. In the future, we should change the tests to run in this locale specifically DDFFORM-417
kasperg
approved these changes
Apr 7, 2024
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.
👍 LGTM
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Link to issue
Jira: DDFFORM-417
This PR depends on two PRs.
Design system PR
CMS PR
Description
This PR adds an app OpeningHours that is used to display the opening hours for a branch.
In order to see data, run the wiremock container.
docker compose up
in the react repo.Go through commit by commit and check the code.
I've seperated them for clarity, but they depend on eachother.
Generally just comment on anything you find relevant, but specifically the "Commetns / questions" below.
Screenshot of the result
Additional comments or questions
If you see another way of doing this, or think it should be removed, comment.