Skip to content
This repository has been archived by the owner on Feb 22, 2023. It is now read-only.

Update the VSearchTypeButton to match the new homepage designs #2069

Merged
merged 11 commits into from
Jan 8, 2023

Conversation

obulat
Copy link
Contributor

@obulat obulat commented Dec 26, 2022

Fixes

Related to #1433 by @panchovm

Description

The main goal of this PR is to make it possible to set whether the search type button has a label in JavaScript, instead of relying on the CSS breakpoint. This is necessary because the breakpoints from which the button has a text label (in addition to the icon) are different: lg on the homepage and xl in the header.

Here is a list of all changes in this PR:

  • it adds a prop for showing or hiding the text label, instead of relying on the screen width.
  • to stop the button from changing the width when the label text changes, it adds all of the non-selected search type labels, but makes them invisible (and aria-hidden). This way, the button width is as high as the longest search type name. It is also capped at 175px (if the text is wider, it is truncated) to make sure that the button doesn't grow larger than 260px (the width of the content switcher popover). This change was reverted because it doesn't work well when the labels have very different lengths:

Screenshot 2022-12-27 at 6 14 35 AM

  • updates the label of the button to "Select the content type: " to make its purpose clearer.
  • removes the Search type buttons from the new mobile homepage, and instead always uses the new VSearchTypeButton inside the homepage search bar.
  • adds all of the necessary props to the activeType in the useSearchType composable.
  • rotates the chevron when the button is pressed to show that it's open. I wanted to try this out because that's what I usually see happen to the dropdown arrow, but I can easily revert it. Should it be rotated as in this PR, or should it stay the same as in Figma, @panchovm? - change reverted, see the reason in a comment below.

Testing Instructions

Run the storybook to see the VSearchTypeButton.
Go to /preferences and turn the new_header flag on. Then, check the VSearchTypeButton in the homepage searchbar, and in the search header. It should look correct, and should have correct aria properties (so, check the Screen reader experience, too).

Checklist

  • My pull request has a descriptive title (not a vague title like
    Update 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.

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.

@github-actions
Copy link

github-actions bot commented Dec 26, 2022

Storybook and Tailwind configuration previews: Ready

Storybook: https://wordpress.github.io/openverse-frontend/_preview/2069
Tailwind: https://wordpress.github.io/openverse-frontend/_preview/2069/tailwind

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.

@openverse-bot openverse-bot added the 🚦 status: awaiting triage Has not been triaged & therefore, not ready for work label Dec 26, 2022
@github-actions
Copy link

github-actions bot commented Dec 26, 2022

Size Change: -100 B (0%)

Total Size: 896 kB

Filename Size Change
./.nuxt/dist/client/app.js 148 kB +56 B (0%)
./.nuxt/dist/client/app.modern.js 123 kB +117 B (0%)
./.nuxt/dist/client/components/v-copy-license/components/v-error-image/components/v-media-reuse/components/v-search-grid/09090664.js 9.87 kB +14 B (0%)
./.nuxt/dist/client/components/v-copy-license/components/v-error-image/components/v-media-reuse/components/v-search-grid/09090664.modern.js 9.84 kB +15 B (0%)
./.nuxt/dist/client/components/v-homepage-content.js 820 B -130 B (-14%) 👏
./.nuxt/dist/client/components/v-homepage-content.modern.js 823 B -130 B (-14%) 👏
./.nuxt/dist/client/components/v-sources-table.js 16.5 kB -29 B (0%)
./.nuxt/dist/client/components/v-sources-table.modern.js 16.6 kB -27 B (0%)
./.nuxt/dist/client/pages/image/_id.js 9.25 kB -18 B (0%)
./.nuxt/dist/client/pages/index.js 7.97 kB +14 B (0%)
ℹ️ View Unchanged
Filename Size Change
./.nuxt/dist/client/236.js 272 B 0 B
./.nuxt/dist/client/236.modern.js 277 B 0 B
./.nuxt/dist/client/237.js 1.85 kB 0 B
./.nuxt/dist/client/commons/app.js 86.7 kB +3 B (0%)
./.nuxt/dist/client/commons/app.modern.js 77.4 kB +7 B (0%)
./.nuxt/dist/client/components/loading-icon.js 747 B 0 B
./.nuxt/dist/client/components/loading-icon.modern.js 748 B 0 B
./.nuxt/dist/client/components/table-sort-icon.js 508 B 0 B
./.nuxt/dist/client/components/table-sort-icon.modern.js 513 B -1 B (0%)
./.nuxt/dist/client/components/v-all-results-grid.js 7.49 kB +1 B (0%)
./.nuxt/dist/client/components/v-all-results-grid.modern.js 5.01 kB +1 B (0%)
./.nuxt/dist/client/components/v-audio-cell.js 357 B 0 B
./.nuxt/dist/client/components/v-audio-cell.modern.js 361 B 0 B
./.nuxt/dist/client/components/v-audio-details.js 2.53 kB -2 B (0%)
./.nuxt/dist/client/components/v-audio-details.modern.js 1.78 kB -1 B (0%)
./.nuxt/dist/client/components/v-audio-track-skeleton.js 1.01 kB +1 B (0%)
./.nuxt/dist/client/components/v-audio-track-skeleton.modern.js 1.02 kB 0 B
./.nuxt/dist/client/components/v-audio-track.js 5.19 kB 0 B
./.nuxt/dist/client/components/v-audio-track.modern.js 5.14 kB -3 B (0%)
./.nuxt/dist/client/components/v-back-to-search-results-link.js 537 B -1 B (0%)
./.nuxt/dist/client/components/v-back-to-search-results-link.modern.js 543 B +1 B (0%)
./.nuxt/dist/client/components/v-bone.js 684 B -1 B (0%)
./.nuxt/dist/client/components/v-bone.modern.js 687 B 0 B
./.nuxt/dist/client/components/v-box-layout.js 1.21 kB +2 B (0%)
./.nuxt/dist/client/components/v-box-layout.modern.js 1.2 kB 0 B
./.nuxt/dist/client/components/v-content-link.js 1.11 kB 0 B
./.nuxt/dist/client/components/v-content-link.modern.js 1.09 kB 0 B
./.nuxt/dist/client/components/v-content-page.js 467 B 0 B
./.nuxt/dist/client/components/v-content-page.modern.js 471 B 0 B
./.nuxt/dist/client/components/v-content-report-button.js 778 B -1 B (0%)
./.nuxt/dist/client/components/v-content-report-button.modern.js 779 B 0 B
./.nuxt/dist/client/components/v-content-report-form.js 6.08 kB +3 B (0%)
./.nuxt/dist/client/components/v-content-report-form.modern.js 3.57 kB 0 B
./.nuxt/dist/client/components/v-content-report-popover.js 1.22 kB 0 B
./.nuxt/dist/client/components/v-content-report-popover.modern.js 4.22 kB -2 B (0%)
./.nuxt/dist/client/components/v-copy-button.js 3.99 kB +2 B (0%)
./.nuxt/dist/client/components/v-copy-button.modern.js 3.99 kB 0 B
./.nuxt/dist/client/components/v-copy-license.js 1 kB 0 B
./.nuxt/dist/client/components/v-copy-license.modern.js 1 kB 0 B
./.nuxt/dist/client/components/v-dmca-notice.js 749 B 0 B
./.nuxt/dist/client/components/v-dmca-notice.modern.js 751 B -1 B (0%)
./.nuxt/dist/client/components/v-error-image.js 1.7 kB +4 B (0%)
./.nuxt/dist/client/components/v-error-image.modern.js 1.68 kB -4 B (0%)
./.nuxt/dist/client/components/v-error-section.js 372 B -1 B (0%)
./.nuxt/dist/client/components/v-error-section.modern.js 375 B -1 B (0%)
./.nuxt/dist/client/components/v-external-search-form.js 3.09 kB -1 B (0%)
./.nuxt/dist/client/components/v-external-search-form.modern.js 3.06 kB 0 B
./.nuxt/dist/client/components/v-external-source-list.js 2.55 kB -1 B (0%)
./.nuxt/dist/client/components/v-external-source-list.modern.js 2.52 kB 0 B
./.nuxt/dist/client/components/v-full-layout.js 1.59 kB -2 B (0%)
./.nuxt/dist/client/components/v-full-layout.modern.js 1.59 kB +1 B (0%)
./.nuxt/dist/client/components/v-grid-skeleton.js 1.61 kB +3 B (0%)
./.nuxt/dist/client/components/v-grid-skeleton.modern.js 1.62 kB -2 B (0%)
./.nuxt/dist/client/components/v-home-gallery.js 4.8 kB 0 B
./.nuxt/dist/client/components/v-home-gallery.modern.js 4.79 kB 0 B
./.nuxt/dist/client/components/v-image-carousel.js 4.76 kB -1 B (0%)
./.nuxt/dist/client/components/v-image-carousel.modern.js 4.74 kB +3 B (0%)
./.nuxt/dist/client/components/v-image-cell-square.js 993 B 0 B
./.nuxt/dist/client/components/v-image-cell-square.modern.js 997 B +1 B (0%)
./.nuxt/dist/client/components/v-image-cell.js 1.43 kB 0 B
./.nuxt/dist/client/components/v-image-cell.modern.js 1.42 kB -1 B (0%)
./.nuxt/dist/client/components/v-image-details.js 2.14 kB -2 B (0%)
./.nuxt/dist/client/components/v-image-details.modern.js 1.42 kB +1 B (0%)
./.nuxt/dist/client/components/v-image-grid.js 4.88 kB -1 B (0%)
./.nuxt/dist/client/components/v-image-grid.modern.js 2.42 kB +3 B (0%)
./.nuxt/dist/client/components/v-license-tab-panel.js 521 B 0 B
./.nuxt/dist/client/components/v-license-tab-panel.modern.js 525 B -1 B (0%)
./.nuxt/dist/client/components/v-load-more.js 3.16 kB +2 B (0%)
./.nuxt/dist/client/components/v-load-more.modern.js 683 B 0 B
./.nuxt/dist/client/components/v-media-license.js 819 B +1 B (0%)
./.nuxt/dist/client/components/v-media-license.modern.js 828 B 0 B
./.nuxt/dist/client/components/v-media-reuse.js 1.62 kB -2 B (0%)
./.nuxt/dist/client/components/v-media-reuse.modern.js 1.61 kB -1 B (0%)
./.nuxt/dist/client/components/v-media-tag.js 428 B 0 B
./.nuxt/dist/client/components/v-media-tag.modern.js 433 B 0 B
./.nuxt/dist/client/components/v-no-results.js 2.75 kB 0 B
./.nuxt/dist/client/components/v-no-results.modern.js 2.72 kB 0 B
./.nuxt/dist/client/components/v-old-homepage-content.js 1.88 kB 0 B
./.nuxt/dist/client/components/v-old-homepage-content.modern.js 1.85 kB -2 B (0%)
./.nuxt/dist/client/components/v-radio.js 1.51 kB +1 B (0%)
./.nuxt/dist/client/components/v-radio.modern.js 1.47 kB 0 B
./.nuxt/dist/client/components/v-related-audio.js 1.25 kB 0 B
./.nuxt/dist/client/components/v-related-audio.modern.js 1.25 kB +1 B (0%)
./.nuxt/dist/client/components/v-related-images.js 1.05 kB -1 B (0%)
./.nuxt/dist/client/components/v-related-images.modern.js 2.98 kB +2 B (0%)
./.nuxt/dist/client/components/v-report-desc-form.js 965 B 0 B
./.nuxt/dist/client/components/v-report-desc-form.modern.js 969 B 0 B
./.nuxt/dist/client/components/v-row-layout.js 1.7 kB -1 B (0%)
./.nuxt/dist/client/components/v-row-layout.modern.js 1.71 kB -1 B (0%)
./.nuxt/dist/client/components/v-scroll-button.js 813 B 0 B
./.nuxt/dist/client/components/v-scroll-button.modern.js 819 B 0 B
./.nuxt/dist/client/components/v-search-grid.js 5.43 kB -1 B (0%)
./.nuxt/dist/client/components/v-search-grid.modern.js 5.38 kB -1 B (0%)
./.nuxt/dist/client/components/v-search-results-title.js 656 B +1 B (0%)
./.nuxt/dist/client/components/v-search-results-title.modern.js 655 B -1 B (0%)
./.nuxt/dist/client/components/v-search-type-radio.js 792 B 0 B
./.nuxt/dist/client/components/v-search-type-radio.modern.js 768 B -2 B (0%)
./.nuxt/dist/client/components/v-server-timeout.js 299 B 0 B
./.nuxt/dist/client/components/v-server-timeout.modern.js 303 B 0 B
./.nuxt/dist/client/components/v-sketch-fab-viewer.js 3.37 kB +1 B (0%)
./.nuxt/dist/client/components/v-sketch-fab-viewer.modern.js 895 B +1 B (0%)
./.nuxt/dist/client/components/v-skip-to-content-container.js 888 B +3 B (0%)
./.nuxt/dist/client/components/v-skip-to-content-container.modern.js 894 B 0 B
./.nuxt/dist/client/components/v-snackbar.js 1.18 kB +1 B (0%)
./.nuxt/dist/client/components/v-snackbar.modern.js 1.19 kB 0 B
./.nuxt/dist/client/components/v-warning-suppressor.js 299 B 0 B
./.nuxt/dist/client/components/v-warning-suppressor.modern.js 302 B -1 B (0%)
./.nuxt/dist/client/pages/about.js 1.51 kB 0 B
./.nuxt/dist/client/pages/about.modern.js 1.51 kB +1 B (0%)
./.nuxt/dist/client/pages/audio/_id.js 7.96 kB 0 B
./.nuxt/dist/client/pages/audio/_id.modern.js 4.79 kB +4 B (0%)
./.nuxt/dist/client/pages/external-sources.js 1.53 kB +3 B (0%)
./.nuxt/dist/client/pages/external-sources.modern.js 1.53 kB 0 B
./.nuxt/dist/client/pages/feedback.js 1.31 kB -1 B (0%)
./.nuxt/dist/client/pages/feedback.modern.js 1.31 kB 0 B
./.nuxt/dist/client/pages/image/_id.modern.js 7.33 kB +2 B (0%)
./.nuxt/dist/client/pages/index.modern.js 7.86 kB +9 B (0%)
./.nuxt/dist/client/pages/preferences.js 1.22 kB -1 B (0%)
./.nuxt/dist/client/pages/preferences.modern.js 1.2 kB 0 B
./.nuxt/dist/client/pages/privacy.js 983 B +1 B (0%)
./.nuxt/dist/client/pages/privacy.modern.js 984 B 0 B
./.nuxt/dist/client/pages/search-help.js 1.61 kB +1 B (0%)
./.nuxt/dist/client/pages/search-help.modern.js 1.61 kB 0 B
./.nuxt/dist/client/pages/search.js 5.09 kB -1 B (0%)
./.nuxt/dist/client/pages/search.modern.js 2.6 kB -1 B (0%)
./.nuxt/dist/client/pages/search/audio.js 6.14 kB +2 B (0%)
./.nuxt/dist/client/pages/search/audio.modern.js 3.65 kB -3 B (0%)
./.nuxt/dist/client/pages/search/image.js 661 B 0 B
./.nuxt/dist/client/pages/search/image.modern.js 2.73 kB +1 B (0%)
./.nuxt/dist/client/pages/search/index.js 542 B -1 B (0%)
./.nuxt/dist/client/pages/search/index.modern.js 548 B 0 B
./.nuxt/dist/client/pages/search/model-3d.js 243 B 0 B
./.nuxt/dist/client/pages/search/model-3d.modern.js 246 B 0 B
./.nuxt/dist/client/pages/search/search-page.types.js 266 B 0 B
./.nuxt/dist/client/pages/search/search-page.types.modern.js 271 B 0 B
./.nuxt/dist/client/pages/search/video.js 240 B 0 B
./.nuxt/dist/client/pages/search/video.modern.js 244 B +1 B (0%)
./.nuxt/dist/client/pages/sources.js 1.55 kB -1 B (0%)
./.nuxt/dist/client/pages/sources.modern.js 1.54 kB +1 B (0%)
./.nuxt/dist/client/runtime.js 2.73 kB 0 B
./.nuxt/dist/client/runtime.modern.js 2.73 kB 0 B
./.nuxt/dist/client/vendors/app.js 63.7 kB -1 B (0%)
./.nuxt/dist/client/vendors/app.modern.js 63.1 kB -4 B (0%)

compressed-size-action

@obulat obulat force-pushed the update/search_type_button branch 2 times, most recently from 173b094 to 7e8b808 Compare December 26, 2022 09:07
@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 🚦 status: awaiting triage Has not been triaged & therefore, not ready for work labels Dec 26, 2022
@fcoveram
Copy link

Rotates the chevron when the button is pressed to show that it's open. I wanted to try this out because that's what I usually see happen to the dropdown arrow, but I can easily revert it. Should it be rotated as in this PR, or should it stay the same as in Figma?

The chevron rotation is mostly for indicating areas that expand/collapse. Like Gutenberg does.

But in this case, the chevron lives in a button that opens a popover. Like Github does.

Therefore, I would keep the chevron orientation as in the mockup.

@obulat obulat force-pushed the update/search_type_button branch from 6fe9e98 to 805c716 Compare December 27, 2022 10:02
@obulat obulat marked this pull request as ready for review December 27, 2022 10:04
@obulat obulat requested a review from a team as a code owner December 27, 2022 10:04
@obulat obulat requested review from zackkrida and dhruvkb December 27, 2022 10:04
@obulat obulat force-pushed the update/search_type_button branch 3 times, most recently from 1d1ce19 to fd4f474 Compare January 2, 2023 15:21
@WordPress WordPress deleted a comment from github-actions bot Jan 3, 2023
@obulat obulat force-pushed the update/search_type_button branch from a037637 to 346287e Compare January 3, 2023 04:41
@obulat obulat requested a review from fcoveram January 3, 2023 06:11
@obulat obulat changed the base branch from main to fix/internal_header_spacings January 3, 2023 08:20
@obulat obulat force-pushed the update/search_type_button branch from 346287e to 11fa9e7 Compare January 3, 2023 08:24
@obulat obulat force-pushed the fix/internal_header_spacings branch from 06818c1 to c9b691c Compare January 3, 2023 09:03
@obulat obulat force-pushed the update/search_type_button branch 2 times, most recently from a2865a3 to 9e21abe Compare January 3, 2023 11:23
@WordPress WordPress deleted a comment from github-actions bot Jan 3, 2023
Copy link

@fcoveram fcoveram left a comment

Choose a reason for hiding this comment

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

It looks excellent ✨

I tested it with VoiceOver and the label was read correctly. Although I noticed one style error:

  • The "showLabel = true" variant needs to have right padding of 12px instead of 8px.

Base automatically changed from fix/internal_header_spacings to main January 4, 2023 02:09
@obulat obulat force-pushed the update/search_type_button branch from 9e21abe to 5373263 Compare January 4, 2023 11:06
@obulat obulat requested a review from fcoveram January 4, 2023 14:12
@obulat obulat force-pushed the update/search_type_button branch 2 times, most recently from 5f63b65 to 2e9f60e Compare January 5, 2023 10:10
Copy link
Member

@dhruvkb dhruvkb left a comment

Choose a reason for hiding this comment

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

Looks good and works very well.

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.

Really great; lgtm.

@obulat obulat dismissed fcoveram’s stale review January 7, 2023 04:13

The padding was fixed

@obulat obulat force-pushed the update/search_type_button branch from 20409db to 25a5942 Compare January 7, 2023 04:27
@obulat obulat force-pushed the update/search_type_button branch from 498c7dd to 5106f6c Compare January 8, 2023 02:53
@@ -317,7 +317,7 @@ a.button {
}

.action-menu {
@apply border-tx bg-tx text-dark-charcoal hover:border-dark-charcoal-20;
@apply border-tx bg-white text-dark-charcoal hover:border-dark-charcoal-20 group-focus-within:hover:border-tx;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Suggested change
@apply border-tx bg-white text-dark-charcoal hover:border-dark-charcoal-20 group-focus-within:hover:border-tx;
@apply border-tx bg-white text-dark-charcoal hover:border-dark-charcoal-20;

@obulat obulat merged commit 26fb744 into main Jan 8, 2023
@obulat obulat deleted the update/search_type_button branch January 8, 2023 03:33
github-actions bot pushed a commit that referenced this pull request Jan 8, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants