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

Spacing adjustments #2231

Merged
merged 2 commits into from
Dec 17, 2024
Merged

Conversation

brianfleming
Copy link
Contributor

@brianfleming brianfleming commented Dec 16, 2024

Description

Some layout changes to make more room for UI elements. There has been some feedback after the last tabs changes, this further refines layout to make things more consistent:

  • adjust min widths for media items to increase the chance tabs will not wrap
  • use the youtube logic to set the min and max width of the large media card item so it matches their list view
  • more predictable media page responsive layout change by specifically change the flex direction at the breakpoint

Unrelated:

  • noticed a weird wording in a toast message, so changed it.

Type of change

  • Performance improvement and/or refactoring (non-breaking change that keeps existing functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Security mitigation or enhancement
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Automated test (add or update automated tests)

How has this been tested?

visited all major page layouts in app to verify consistent margin/padding

Things to pay attention to during code review

  • test out different screen sizes on media page to make sure it all makes sense
  • all of the major containers in the app have gone from 32px side margin/padding to 16px which is more consistent for other UI elements

Checklist

  • I have performed a self-review of my own code
  • I've made sure my branch is runnable and given good testing steps in the PR description
  • I considered secure coding practices when writing this code. Any security concerns are noted above.
  • I have commented my code in hard-to-understand areas, if any
  • I have made needed changes to the README
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • If I implemented any new components, they are self-contained, their propTypes are declared and they use React Hooks and, if data-fetching is required, they use Relay Modern with fragment containers
  • If my components involve user interaction - specifically button, text fields, or other inputs - I have added a BEM-like class name to the element that is interacted with
  • To the best of my knowledge, any new styles are applied according to the design system
  • If I added a new external dependency, I included a rationale for doing so and an estimate of the change in bundle size (e.g., checked in https://bundlephobia.com/)
  • If I touched a file that included an eslint-disable-file header, I updated the code such that the disabler can be removed

… some tabs feedback, adjust minor wording in toast message
@@ -19,7 +19,7 @@
}

.listMainAction {
padding: 0 18px;
padding: 0 16px;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed atypical use of 18 vs 16px

@brianfleming brianfleming marked this pull request as ready for review December 16, 2024 21:09
@brianfleming
Copy link
Contributor Author

cc @caiosba this was the PR i mentioned during our 1:1

@caiosba
Copy link
Contributor

caiosba commented Dec 16, 2024

Thanks, @brianfleming - @sarahkeh , can you please take the first review and ask for @amoedoamorim to do a second review if needed? We want to get these fixes in at the same time as the other fixes I'm going to deploy for the data dashboard.

Copy link
Contributor

@sarahkeh sarahkeh left a comment

Choose a reason for hiding this comment

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

lgtm

@brianfleming brianfleming merged commit 33fce3b into develop Dec 17, 2024
11 checks passed
@brianfleming brianfleming deleted the brianfleming/dev/minor-adjustments branch December 17, 2024 00:13
caiosba pushed a commit that referenced this pull request Dec 18, 2024
* adjustments to some spacing to make more room for UI as a response to some tabs feedback, adjust minor wording in toast message

* simplify some sizing and use the youtube min max list size for large media card
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.

3 participants