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

Style/#289 acknowledgement page responsive css #293

Merged
merged 11 commits into from
Aug 30, 2023

Conversation

daniel-cy-lu
Copy link
Contributor

@daniel-cy-lu daniel-cy-lu commented Aug 17, 2023

Acknowledgement Page, which can be found via the link in the footer. Responsive CSS completed for mobile, tablet and desktop.

Refer to Patrick's mock up: https://www.figma.com/proto/fXh58MfxEDh1ssgxQyPb5f/Overture_Refresh?type=design&node-id=1159-10536&t=B0eRBTRc8HE2PXBM-0&scaling=min-zoom&page-id=1098%3A3803&starting-point-node-id=1167%3A12310

Patrick has no MVP feedback for this page.

@daniel-cy-lu daniel-cy-lu changed the title WIP Style/#289 acknowledgement page responsive css Style/#289 acknowledgement page responsive css Aug 17, 2023
@@ -0,0 +1,34 @@
import React from 'react';
Copy link
Contributor Author

@daniel-cy-lu daniel-cy-lu Aug 28, 2023

Choose a reason for hiding this comment

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

This file is of the next ticket, Products Page Update. Not sure why it appears in this ticket. I made sure I closed old tabs and then switch local branches.

@@ -0,0 +1,58 @@
@import '../../styles/colors';
Copy link
Contributor Author

@daniel-cy-lu daniel-cy-lu Aug 28, 2023

Choose a reason for hiding this comment

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

Belong to next ticket, Products Page Update

@ciaranschutte
Copy link
Contributor

@daniel-cy-lu curious why we're moving away from using the Hero component. It's a really good encapsulation of that bit of display. Not using it means we're back to having repeating chunks of very similar code with only slight differences... which is not good (not reusable, hard to maintain). Will msg you on slack too as a conversation might be easier.

@daniel-cy-lu
Copy link
Contributor Author

@daniel-cy-lu curious why we're moving away from using the Hero component. It's a really good encapsulation of that bit of display. Not using it means we're back to having repeating chunks of very similar code with only slight differences... which is not good (not reusable, hard to maintain). Will msg you on slack too as a conversation might be easier.

This discussion extended to a zoom chat, and we decided to use a modified Hero component best suited for the current version of the Hero. Cleaned up Hero component and its css.

@daniel-cy-lu
Copy link
Contributor Author

@ciaranschutte I cleaned up all the comments and found a lot of unused css in the case study page, and got rid of those as well.

@daniel-cy-lu
Copy link
Contributor Author

daniel-cy-lu commented Aug 29, 2023

Because we modified Hero component, currently Services and About Us pages don't have a Hero image nor css applied. These two pages however are going to receive complete re-design.
The Hero will be constructed using the new Hero component.

@ciaranschutte
Copy link
Contributor

src/pages/acknowledgements/assets/img_hero_acknowledgements_full.svg
src/pages/case-studies/assets/img_hero_case_studies.svg
are these the same svgs? can remove duplication

@ciaranschutte
Copy link
Contributor

@daniel-cy-lu looks great. nice cleanup. just ensure that's not duplicate image then good to go!

@ciaranschutte
Copy link
Contributor

@daniel-cy-lu couple of things I noticed in reviewing, but aren't changes in this PR, so I've made a ticket to detail them: #305
I'll leave it to your project leaders to decide if / when to tackle this

@daniel-cy-lu
Copy link
Contributor Author

daniel-cy-lu commented Aug 30, 2023

@ciaranschutte I read the new ticket, I will double check the font size and font of <h1>, <h2>, <h3>. The idea was so the project match exact what is in the mock up.

@daniel-cy-lu daniel-cy-lu merged commit 8ba141d into develop Aug 30, 2023
5 checks passed
@daniel-cy-lu daniel-cy-lu deleted the style/#289-acknowledgement-page-responsive-css branch August 30, 2023 13:42
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