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.

Sign up for GitHub

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

Jump to bottom

solve #495 render all page headers on backend remove react helmet component #1018

Merged
merged 5 commits into from Nov 2, 2023
Merged

solve #495 render all page headers on backend remove react helmet component #1018

merged 5 commits into from Nov 2, 2023

Conversation

QuanGiap
Copy link
Collaborator

No description provided.

@QuanGiap QuanGiap added ready for review Full Stack Both front- and back-end work required labels Aug 16, 2023
@QuanGiap QuanGiap self-assigned this Aug 16, 2023
@QuanGiap QuanGiap linked an issue Aug 16, 2023 that may be closed by this pull request
Render all page headers on backend, remove react-helmet component #495
Closed
3 tasks
PeterBreen
PeterBreen requested changes Aug 17, 2023
View reviewed changes
Copy link
Collaborator

@PeterBreen PeterBreen left a comment

Choose a reason for hiding this comment

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

I tested this for a little bit in my docker setup this morning - it looks pretty good, thank you! I've got a few line comments where there's some code we might not need and one grammar cleanup item. Otherwise, it seems to work just fine. I checked the changed pages and they're showing the correct page header info.

civictechprojects/helpers/context_preload.py Outdated Show resolved Hide resolved
civictechprojects/helpers/context_preload.py Outdated Show resolved Hide resolved
common/components/controllers/LogInController.jsx Outdated Show resolved Hide resolved
civictechprojects/helpers/context_preload.py Outdated Show resolved Hide resolved
@QuanGiap
Copy link
Collaborator Author

QuanGiap commented Aug 18, 2023 •
edited

I tested this for a little bit in my docker setup this morning - it looks pretty good, thank you! I've got a few line comments where there's some code we might not need and one grammar cleanup item. Otherwise, it seems to work just fine. I checked the changed pages and they're showing the correct page header info.

Hello, thanks for reviewing the code and giving suggestions. It should be fine to delete the press page preload, I didn't know that the press page was deleted. I have fixed the code base on your suggestion

@PeterBreen PeterBreen mentioned this pull request Aug 31, 2023
Open
48 tasks
PeterBreen
PeterBreen previously approved these changes Aug 31, 2023
View reviewed changes
@marlonkeating marlonkeating changed the base branch from master to sprint-95-release November 2, 2023 01:30
@marlonkeating marlonkeating merged commit 53978c5 into sprint-95-release Nov 2, 2023
This was referenced Nov 2, 2023
Closed
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@marlonkeating marlonkeating marlonkeating approved these changes

@PeterBreen PeterBreen PeterBreen left review comments

Assignees

@QuanGiap QuanGiap

Labels
Full Stack Both front- and back-end work required ready for review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Render all page headers on backend, remove react-helmet component
3 participants
@QuanGiap @marlonkeating @PeterBreen