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

UHF-6668 Add listing events in React #365

Conversation

jeremysteerio
Copy link
Contributor

@jeremysteerio jeremysteerio commented Sep 27, 2022

Events list in React UHF-6668

First step in 'Reactifying' the events paragraph

What was done

  • Added React app for dispaying events paragraph
  • Modified events paragraph template to include React app

How to install

  • Set up any instance
  • Get this branch composer require drupal/helfi_platform_config:dev-UHF-6668-linkedevents-react-results
  • Get HDBT source composer reinstall drupal/hdbt --prefer-source
  • Cherry pick commit from HDBT
    • cd public/theme/contrib/hdbt
    • git cherry-pick 02ceb46c8fbbf81447d3a7b19a5738a38b5a3b30
  • Run drush-cr
  • Add an events paragraph to a landing or standard page

How to test

@Jussiles Jussiles self-requested a review September 28, 2022 07:25
@jeremysteerio jeremysteerio changed the title Add listing events in React UHF-6668 Add listing events in React Sep 28, 2022
Copy link
Contributor

@Jussiles Jussiles left a comment

Choose a reason for hiding this comment

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

Looks good! Added couple of comments about coding style things. I also noticed that on mobile view tag list is on top of the image althought in the designs is under the location. Is it like that for a reason?

"i18next-browser-languagedetector": "^6.1.2",
"react": "^17.0.2",
"react-dom": "^17.0.2",
"react-i18next": "^11.7.3",
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe i18next packages could be moved if you're not using those.

})
.catch(e => setFailed(true))
.finally(() => setLoading(false))
;
Copy link
Contributor

Choose a reason for hiding this comment

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

coding style fix here

street_address?: MultilingualString
};

export default Event;
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe not every type needs to exported if not imported in other files. Also Event has double export.

@jeremysteerio jeremysteerio changed the base branch from main to UHF-3128-sote-linkedevents-filtering October 3, 2022 06:34
@jeremysteerio jeremysteerio merged commit 9368db7 into UHF-3128-sote-linkedevents-filtering Oct 3, 2022
@jeremysteerio jeremysteerio deleted the UHF-6668-linkedevents-react-results branch October 3, 2022 07:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants