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

[Improvement] Add padding to internal Layout component, move useSteps hook, misc small fixes #32

Merged
merged 4 commits into from
Jul 23, 2021
Merged

[Improvement] Add padding to internal Layout component, move useSteps hook, misc small fixes #32

merged 4 commits into from
Jul 23, 2021

Conversation

OustanDing
Copy link
Member

@OustanDing OustanDing commented Jul 22, 2021 •
edited
Loading

Notion ticket link

N/A

Implementation description

  • Previously, we were manually adding marginTop=64px to the internal pages. This was incorrect - it should've been 32px. Also, the 32px is consistent across all admin pages. Moved the 32px spacing as padding to the internal Layout component
  • Changed marginY to paddingY in the applicant Layout component for consistency
  • Moved useSteps.ts from tools/components/Steps/ to tools/hooks/
  • Removed prisma format from pre-commit hook to remove unnecessary diffs

Notes

  • Please rebase frontend feature PRs after this gets merged!

Checklist

  • My PR name is descriptive, is in imperative tense and starts with one of the following: [Feature],[Improvement] or [Fix],
  • I have run the appropriate linter(s)
  • I have requested a review from the RCD team on GitHub, or specific people who are associated with this ticket

@OustanDing OustanDing added the needs dev review Needs dev review label Jul 22, 2021
@OustanDing OustanDing requested a review from a team July 22, 2021 18:35
@railway-app
Copy link

railway-app bot commented Jul 22, 2021 •
edited
Loading

This PR is being deployed to Railway 🚅

Live → richmond-centre-for-disability-pr-32.up.railway.app

[View Deployment]

@OustanDing OustanDing changed the title [Improvement] Add top and bottom padding to internal Layout component [Improvement] Add padding to internal Layout component, move useSteps hook Jul 22, 2021
EmilioMena
EmilioMena approved these changes Jul 22, 2021
View reviewed changes
Copy link
Contributor

@EmilioMena EmilioMena left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Nice! Looks good

@EmilioMena
Copy link
Contributor

Please rebase frontend feature PRs after this gets merged!

Might be a good idea to broadcast this on slack so that everyone gets the heads up 📢

@OustanDing
Copy link
Member Author

Thanks @EmilioMena and @jeffreyzhang2001 for the review! Will make an announcement right now

@OustanDing OustanDing changed the title [Improvement] Add padding to internal Layout component, move useSteps hook [Improvement] Add padding to internal Layout component, move useSteps hook, misc small fixes Jul 23, 2021
@OustanDing OustanDing merged commit a722ecc into staging Jul 23, 2021
@OustanDing OustanDing deleted the od/improvement/add-margins-to-internal-layout branch July 23, 2021 20:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jeffreyzhang2001 jeffreyzhang2001 jeffreyzhang2001 approved these changes

@EmilioMena EmilioMena EmilioMena approved these changes

Assignees
No one assigned
Labels
needs dev review Needs dev review
Projects
None yet
Milestone
No milestone
3 participants
@OustanDing @EmilioMena @jeffreyzhang2001