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

Spacing improvements in All content's <header> area #2084

Merged
merged 2 commits into from
May 16, 2023
Merged

Spacing improvements in All content's <header> area #2084

merged 2 commits into from
May 16, 2023

Conversation

obulat
Copy link
Contributor

@obulat obulat commented May 12, 2023 •
edited
Loading

Fixes

Fixes #2079 by @fcoveram

Description

This PR adds the following spacing and text style changes from the #2079 to improve the layout:

  • Reduce the line height of Heading 1 text style from 1.3 to 1.
  • Apply Heading 2 text style to <h1> on md breakpoint.
  • Change the paddings of <header> element where h1 is placed to be 8px for top and 24px for bottom.
  • Hide the on xs and sm as the element adds unnecessary space.

Testing Instructions

Check that the "All content" header area matches the designs all layouts in Views page in the Design Library for reference.

Checklist

  • My pull request has a descriptive title (not a vague title likeUpdate index.md).
  • My pull request targets the default branch of the repository (main) or a parent feature branch.
  • My commit messages follow best practices.
  • My code follows the established code style of the repository.
  • I added or updated tests for the changes I made (if applicable).
  • I added or updated documentation (if applicable).
  • I tried running the project locally and verified that there are no visible errors.
  • I ran the DAG documentation generator (if applicable).

Developer Certificate of Origin

Developer Certificate of Origin 
Developer Certificate of Origin
Version 1.1

Copyright (C) 2004, 2006 The Linux Foundation and its contributors.
1 Letterman Drive
Suite D4700
San Francisco, CA, 94129

Everyone is permitted to copy and distribute verbatim copies of this
license document, but changing it is not allowed.


Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
    have the right to submit it under the open source license
    indicated in the file; or

(b) The contribution is based upon previous work that, to the best
    of my knowledge, is covered under an appropriate open source
    license and I have the right under that license to submit that
    work with modifications, whether created in whole or in part
    by me, under the same open source license (unless I am
    permitted to submit under a different license), as indicated
    in the file; or

(c) The contribution was provided directly to me by some other
    person who certified (a), (b) or (c) and I have not modified
    it.

(d) I understand and agree that this project and the contribution
    are public and that a record of the contribution (including all
    personal information I submit with it, including my sign-off) is
    maintained indefinitely and may be redistributed consistent with
    this project or the open source license(s) involved.

@obulat obulat requested a review from a team as a code owner May 12, 2023 13:56
@obulat obulat requested review from AetherUnbound and dhruvkb May 12, 2023 13:56
@github-actions github-actions bot added 🧱 stack: documentation Related to Sphinx documentation 🧱 stack: frontend Related to the Nuxt frontend labels May 12, 2023
@github-actions
Copy link

github-actions bot commented May 12, 2023 •
edited
Loading

Size Change: +184 B (0%)

Total Size: 857 kB

Filename Size Change
./frontend/.nuxt/dist/client/app.js 126 kB +33 B (0%)
./frontend/.nuxt/dist/client/app.modern.js 117 kB +34 B (0%)
./frontend/.nuxt/dist/client/components/v-search-grid.js 6.02 kB +13 B (0%)
./frontend/.nuxt/dist/client/components/v-search-grid.modern.js 5.94 kB +14 B (0%)
./frontend/.nuxt/dist/client/components/v-search-results-title.js 619 B +17 B (+3%)
./frontend/.nuxt/dist/client/components/v-search-results-title.modern.js 622 B +16 B (+3%)
./frontend/.nuxt/dist/client/components/v-sources-table.js 15.2 kB +29 B (0%)
./frontend/.nuxt/dist/client/components/v-sources-table.modern.js 15.2 kB +28 B (0%)
ℹ️ View Unchanged
Filename Size
./frontend/.nuxt/dist/client/235.js 273 B
./frontend/.nuxt/dist/client/235.modern.js 278 B
./frontend/.nuxt/dist/client/236.js 1.85 kB
./frontend/.nuxt/dist/client/commons/app.js 90.3 kB
./frontend/.nuxt/dist/client/commons/app.modern.js 78.5 kB
./frontend/.nuxt/dist/client/components/loading-icon.js 765 B
./frontend/.nuxt/dist/client/components/loading-icon.modern.js 770 B
./frontend/.nuxt/dist/client/components/table-sort-icon.js 514 B
./frontend/.nuxt/dist/client/components/table-sort-icon.modern.js 519 B
./frontend/.nuxt/dist/client/components/v-all-results-grid.js 5.63 kB
./frontend/.nuxt/dist/client/components/v-all-results-grid.modern.js 5.52 kB
./frontend/.nuxt/dist/client/components/v-audio-cell.js 390 B
./frontend/.nuxt/dist/client/components/v-audio-cell.modern.js 397 B
./frontend/.nuxt/dist/client/components/v-audio-details.js 2.91 kB
./frontend/.nuxt/dist/client/components/v-audio-details.modern.js 2.33 kB
./frontend/.nuxt/dist/client/components/v-audio-details/components/v-content-report-form/components/v-content-report-popover/comp/e356c88c.js 3.85 kB
./frontend/.nuxt/dist/client/components/v-audio-details/components/v-content-report-popover/components/v-image-details/pages/audi/7832d35f.modern.js 4.13 kB
./frontend/.nuxt/dist/client/components/v-audio-track-skeleton.js 1.02 kB
./frontend/.nuxt/dist/client/components/v-audio-track-skeleton.modern.js 1.02 kB
./frontend/.nuxt/dist/client/components/v-audio-track.js 5.61 kB
./frontend/.nuxt/dist/client/components/v-audio-track.modern.js 5.54 kB
./frontend/.nuxt/dist/client/components/v-back-to-search-results-link.js 1.23 kB
./frontend/.nuxt/dist/client/components/v-back-to-search-results-link.modern.js 1.21 kB
./frontend/.nuxt/dist/client/components/v-bone.js 688 B
./frontend/.nuxt/dist/client/components/v-bone.modern.js 693 B
./frontend/.nuxt/dist/client/components/v-box-layout.js 1.28 kB
./frontend/.nuxt/dist/client/components/v-box-layout.modern.js 1.27 kB
./frontend/.nuxt/dist/client/components/v-content-link.js 977 B
./frontend/.nuxt/dist/client/components/v-content-link.modern.js 978 B
./frontend/.nuxt/dist/client/components/v-content-page.js 524 B
./frontend/.nuxt/dist/client/components/v-content-page.modern.js 528 B
./frontend/.nuxt/dist/client/components/v-content-report-button.js 493 B
./frontend/.nuxt/dist/client/components/v-content-report-button.modern.js 497 B
./frontend/.nuxt/dist/client/components/v-content-report-form.js 781 B
./frontend/.nuxt/dist/client/components/v-content-report-form.modern.js 4.04 kB
./frontend/.nuxt/dist/client/components/v-content-report-popover.js 1.49 kB
./frontend/.nuxt/dist/client/components/v-content-report-popover.modern.js 762 B
./frontend/.nuxt/dist/client/components/v-copy-button.js 4 kB
./frontend/.nuxt/dist/client/components/v-copy-button.modern.js 4 kB
./frontend/.nuxt/dist/client/components/v-copy-license.js 1.71 kB
./frontend/.nuxt/dist/client/components/v-copy-license.modern.js 1.69 kB
./frontend/.nuxt/dist/client/components/v-copy-license/components/v-error-image/components/v-media-reuse/components/v-search-grid/d219393b.js 10 kB
./frontend/.nuxt/dist/client/components/v-copy-license/components/v-error-image/components/v-media-reuse/components/v-search-grid/d219393b.modern.js 10 kB
./frontend/.nuxt/dist/client/components/v-dmca-notice.js 791 B
./frontend/.nuxt/dist/client/components/v-dmca-notice.modern.js 799 B
./frontend/.nuxt/dist/client/components/v-error-image.js 1.7 kB
./frontend/.nuxt/dist/client/components/v-error-image.modern.js 1.68 kB
./frontend/.nuxt/dist/client/components/v-error-section.js 372 B
./frontend/.nuxt/dist/client/components/v-error-section.modern.js 375 B
./frontend/.nuxt/dist/client/components/v-external-search-form.js 2.45 kB
./frontend/.nuxt/dist/client/components/v-external-search-form.modern.js 2.41 kB
./frontend/.nuxt/dist/client/components/v-external-source-list.js 1.49 kB
./frontend/.nuxt/dist/client/components/v-external-source-list.modern.js 1.46 kB
./frontend/.nuxt/dist/client/components/v-full-layout.js 2.13 kB
./frontend/.nuxt/dist/client/components/v-full-layout.modern.js 2.11 kB
./frontend/.nuxt/dist/client/components/v-grid-skeleton.js 1.61 kB
./frontend/.nuxt/dist/client/components/v-grid-skeleton.modern.js 1.62 kB
./frontend/.nuxt/dist/client/components/v-home-gallery.js 5.28 kB
./frontend/.nuxt/dist/client/components/v-home-gallery.modern.js 5.25 kB
./frontend/.nuxt/dist/client/components/v-homepage-content.js 1.76 kB
./frontend/.nuxt/dist/client/components/v-homepage-content.modern.js 1.73 kB
./frontend/.nuxt/dist/client/components/v-image-carousel.js 4.74 kB
./frontend/.nuxt/dist/client/components/v-image-carousel.modern.js 4.7 kB
./frontend/.nuxt/dist/client/components/v-image-cell.js 1.61 kB
./frontend/.nuxt/dist/client/components/v-image-cell.modern.js 1.6 kB
./frontend/.nuxt/dist/client/components/v-image-details.js 2.53 kB
./frontend/.nuxt/dist/client/components/v-image-details.modern.js 1.96 kB
./frontend/.nuxt/dist/client/components/v-image-grid.js 3.84 kB
./frontend/.nuxt/dist/client/components/v-image-grid.modern.js 3.73 kB
./frontend/.nuxt/dist/client/components/v-license-tab-panel.js 1.24 kB
./frontend/.nuxt/dist/client/components/v-license-tab-panel.modern.js 1.21 kB
./frontend/.nuxt/dist/client/components/v-load-more.js 809 B
./frontend/.nuxt/dist/client/components/v-load-more.modern.js 707 B
./frontend/.nuxt/dist/client/components/v-media-license.js 828 B
./frontend/.nuxt/dist/client/components/v-media-license.modern.js 836 B
./frontend/.nuxt/dist/client/components/v-media-reuse.js 2.31 kB
./frontend/.nuxt/dist/client/components/v-media-reuse.modern.js 2.29 kB
./frontend/.nuxt/dist/client/components/v-media-tag.js 416 B
./frontend/.nuxt/dist/client/components/v-media-tag.modern.js 420 B
./frontend/.nuxt/dist/client/components/v-modal.js 1.01 kB
./frontend/.nuxt/dist/client/components/v-modal.modern.js 995 B
./frontend/.nuxt/dist/client/components/v-no-results.js 1.42 kB
./frontend/.nuxt/dist/client/components/v-no-results.modern.js 1.39 kB
./frontend/.nuxt/dist/client/components/v-radio.js 1.54 kB
./frontend/.nuxt/dist/client/components/v-radio.modern.js 1.5 kB
./frontend/.nuxt/dist/client/components/v-related-audio.js 1.29 kB
./frontend/.nuxt/dist/client/components/v-related-audio.modern.js 1.29 kB
./frontend/.nuxt/dist/client/components/v-related-images.js 1.1 kB
./frontend/.nuxt/dist/client/components/v-related-images.modern.js 1.1 kB
./frontend/.nuxt/dist/client/components/v-report-desc-form.js 977 B
./frontend/.nuxt/dist/client/components/v-report-desc-form.modern.js 979 B
./frontend/.nuxt/dist/client/components/v-row-layout.js 1.68 kB
./frontend/.nuxt/dist/client/components/v-row-layout.modern.js 1.68 kB
./frontend/.nuxt/dist/client/components/v-scroll-button.js 824 B
./frontend/.nuxt/dist/client/components/v-scroll-button.modern.js 828 B
./frontend/.nuxt/dist/client/components/v-server-timeout.js 299 B
./frontend/.nuxt/dist/client/components/v-server-timeout.modern.js 303 B
./frontend/.nuxt/dist/client/components/v-sketch-fab-viewer.js 1.01 kB
./frontend/.nuxt/dist/client/components/v-sketch-fab-viewer.modern.js 913 B
./frontend/.nuxt/dist/client/components/v-snackbar.js 1.16 kB
./frontend/.nuxt/dist/client/components/v-snackbar.modern.js 1.17 kB
./frontend/.nuxt/dist/client/components/v-warning-suppressor.js 306 B
./frontend/.nuxt/dist/client/components/v-warning-suppressor.modern.js 311 B
./frontend/.nuxt/dist/client/pages/about.js 1.4 kB
./frontend/.nuxt/dist/client/pages/about.modern.js 1.4 kB
./frontend/.nuxt/dist/client/pages/audio/_id/index.js 5.59 kB
./frontend/.nuxt/dist/client/pages/audio/_id/index.modern.js 5.03 kB
./frontend/.nuxt/dist/client/pages/external-sources.js 1.55 kB
./frontend/.nuxt/dist/client/pages/external-sources.modern.js 1.56 kB
./frontend/.nuxt/dist/client/pages/feedback.js 1.34 kB
./frontend/.nuxt/dist/client/pages/feedback.modern.js 1.34 kB
./frontend/.nuxt/dist/client/pages/image/_id/index.js 7.09 kB
./frontend/.nuxt/dist/client/pages/image/_id/index.modern.js 6.43 kB
./frontend/.nuxt/dist/client/pages/image/_id/report.js 1.84 kB
./frontend/.nuxt/dist/client/pages/image/_id/report.modern.js 4.71 kB
./frontend/.nuxt/dist/client/pages/index.js 7.33 kB
./frontend/.nuxt/dist/client/pages/index.modern.js 7.27 kB
./frontend/.nuxt/dist/client/pages/preferences.js 1.32 kB
./frontend/.nuxt/dist/client/pages/preferences.modern.js 1.32 kB
./frontend/.nuxt/dist/client/pages/privacy.js 1.01 kB
./frontend/.nuxt/dist/client/pages/privacy.modern.js 1.02 kB
./frontend/.nuxt/dist/client/pages/search-help.js 1.6 kB
./frontend/.nuxt/dist/client/pages/search-help.modern.js 1.58 kB
./frontend/.nuxt/dist/client/pages/search.js 2.2 kB
./frontend/.nuxt/dist/client/pages/search.modern.js 2.04 kB
./frontend/.nuxt/dist/client/pages/search/audio.js 3.66 kB
./frontend/.nuxt/dist/client/pages/search/audio.modern.js 3.55 kB
./frontend/.nuxt/dist/client/pages/search/image.js 544 B
./frontend/.nuxt/dist/client/pages/search/image.modern.js 548 B
./frontend/.nuxt/dist/client/pages/search/index.js 442 B
./frontend/.nuxt/dist/client/pages/search/index.modern.js 448 B
./frontend/.nuxt/dist/client/pages/search/model-3d.js 242 B
./frontend/.nuxt/dist/client/pages/search/model-3d.modern.js 246 B
./frontend/.nuxt/dist/client/pages/search/search-page.types.js 266 B
./frontend/.nuxt/dist/client/pages/search/search-page.types.modern.js 271 B
./frontend/.nuxt/dist/client/pages/search/video.js 240 B
./frontend/.nuxt/dist/client/pages/search/video.modern.js 243 B
./frontend/.nuxt/dist/client/pages/sources.js 1.51 kB
./frontend/.nuxt/dist/client/pages/sources.modern.js 1.51 kB
./frontend/.nuxt/dist/client/runtime.js 2.74 kB
./frontend/.nuxt/dist/client/runtime.modern.js 2.75 kB
./frontend/.nuxt/dist/client/vendors/app.js 64.6 kB
./frontend/.nuxt/dist/client/vendors/app.modern.js 63.8 kB

compressed-size-action

@github-actions
Copy link

Full-stack documentation: https://docs.openverse.org/_preview/2084

Please note that GitHub pages takes a little time to deploy newly pushed code, if the links above don't work or you see old versions, wait 5 minutes and try again.

You can check the GitHub pages deployment action list to see the current status of the deployments.

@obulat obulat force-pushed the fix/all_content_spacing branch 3 times, most recently from 58a150a to 47b3dd0 Compare May 12, 2023 15:04
@obulat obulat added 🟨 priority: medium Not blocking but should be addressed soon ✨ goal: improvement Improvement to an existing user-facing feature 🕹 aspect: interface Concerns end-users' experience with the software and removed 🧱 stack: documentation Related to Sphinx documentation labels May 12, 2023
zackkrida
zackkrida requested changes May 12, 2023
View reviewed changes
Copy link
Member

@zackkrida zackkrida left a comment

Choose a reason for hiding this comment

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

This might need verification from @fcoveram, but the spacing looks really tight to me, especially when:

  • There is a search term with descenders
  • The sidebar is open
CleanShot 2023-05-12 at 14 11 05@2x

@fcoveram
Copy link
Contributor

You're right. It looks very tight. We can add 8px to top and bottom margins to end up with 16px and 32px
respectively.

In any case, the filter sidebar adds a border in the header that cuts off the white space in between, making it look tighter. We will have this odd effect unless we make the header look equal all the time. Here are two screenshots showing my point

Spacing between header and h1 area

Mockup of header with border and without

Spacing between header and h1 area with notes

Mockup of header with border and without, plus spacing notes

16px for top and 32px works well IMO. The visual effect doesn't impact significantly as scrolling down is likely the natural interaction.

@github-actions github-actions bot added the 🧱 stack: documentation Related to Sphinx documentation label May 15, 2023
@obulat obulat force-pushed the fix/all_content_spacing branch 3 times, most recently from 595b73f to dd11cc8 Compare May 15, 2023 13:59
@obulat obulat requested review from fcoveram and zackkrida May 15, 2023 14:15
@obulat obulat force-pushed the fix/all_content_spacing branch from dd11cc8 to 18dfbe3 Compare May 15, 2023 15:53
zackkrida
zackkrida approved these changes May 15, 2023
View reviewed changes
Copy link
Member

@zackkrida zackkrida left a comment

Choose a reason for hiding this comment

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

Fantastic! LGTM.

AetherUnbound
AetherUnbound approved these changes May 15, 2023
View reviewed changes
Copy link
Collaborator

@AetherUnbound AetherUnbound left a comment

Choose a reason for hiding this comment

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

I've confirmed that the changes match for me locally! 🎉

@obulat obulat merged commit a3cdbe0 into main May 16, 2023
@obulat obulat deleted the fix/all_content_spacing branch May 16, 2023 03:18
obulat added a commit that referenced this pull request May 16, 2023
@obulat
Spacing improvements in All content's <header> area (#2084)
710bc5f 
* Adjust content link spacings

* Update snapshots
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AetherUnbound AetherUnbound AetherUnbound approved these changes

@zackkrida zackkrida zackkrida approved these changes

@dhruvkb dhruvkb Awaiting requested review from dhruvkb dhruvkb is a code owner automatically assigned from WordPress/openverse-frontend

@fcoveram fcoveram Awaiting requested review from fcoveram

Assignees
No one assigned
Labels
🕹 aspect: interface Concerns end-users' experience with the software ✨ goal: improvement Improvement to an existing user-facing feature 🟨 priority: medium Not blocking but should be addressed soon 🧱 stack: documentation Related to Sphinx documentation 🧱 stack: frontend Related to the Nuxt frontend
Projects
Openverse PRs
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Spacing improvements in All content's <header> area
4 participants
@obulat @fcoveram @zackkrida @AetherUnbound