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

Unify style of search buttons #9086

Open
Tracked by #1184
RayBB opened this issue Apr 15, 2024 · 13 comments · May be fixed by #9146
Open
Tracked by #1184

Unify style of search buttons #9086

RayBB opened this issue Apr 15, 2024 · 13 comments · May be fixed by #9146
Assignees
Labels
Good First Issue Easy issue. Good for newcomers. [managed] Lead: @cdrini Issues overseen by Drini (Staff: Team Lead & Solr, Library Explorer, i18n) [managed] Needs: Staff / Internal Reviewed a PR but don't have merge powers? Use this. Priority: 3 Issues that we can consider at our leisure. [managed] Type: Feature Request Issue describes a feature or enhancement we'd like to implement. [managed]

Comments

@RayBB
Copy link
Collaborator

RayBB commented Apr 15, 2024

Following #8963 we want to evaluate unifying the styles of the search buttons with something like cta-btn cta-btn--small cta-btn--vanilla etc.

This will potentially also require changing the text input to match the height and look nice. Maybe like the button group from bootstrap here.

Describe the problem that you'd like solved

Proposal & Constraints

Additional context

Stakeholders

@RayBB RayBB added Type: Feature Request Issue describes a feature or enhancement we'd like to implement. [managed] Needs: Triage This issue needs triage. The team needs to decide who should own it, what to do, by when. [managed] Needs: Lead labels Apr 15, 2024
@mekarpeles mekarpeles added Good First Issue Easy issue. Good for newcomers. [managed] Priority: 3 Issues that we can consider at our leisure. [managed] Lead: @jimchamp Issues overseen by Jim (Front-end Lead, BookNotes) [managed] and removed Needs: Triage This issue needs triage. The team needs to decide who should own it, what to do, by when. [managed] Needs: Lead labels Apr 15, 2024
@0simoo
Copy link

0simoo commented Apr 20, 2024

I'd like to work on this!

@RayBB
Copy link
Collaborator Author

RayBB commented Apr 20, 2024

@0simoo I've assigned you. Feel free to create a proof of concept that you think looks nice and we can discuss further from there.

@0simoo
Copy link

0simoo commented Apr 22, 2024

Here are some proof of concepts. Let me know what you think!

screenshot1 screenshot2

@RayBB
Copy link
Collaborator Author

RayBB commented Apr 23, 2024

@0simoo I like both of them a lot actually. A small personal preference for the 2nd one but both would be a great improvement.
Can you join today's community call to share these?

If not, I'll try to share and see what people say!

@0simoo
Copy link

0simoo commented Apr 23, 2024

I should be able to attend today's community call!

@jimchamp
Copy link
Collaborator

jimchamp commented Apr 23, 2024

Thanks @0simoo! As discussed during today's community call:

  • Everybody generally seems to prefer the combined search box and button
  • Search button should have a slightly darker color in order to better indicate that it is indeed a button
    • @mekarpeles suggests trying the same color as the search facet selector (pictured below)

image

@0simoo
Copy link

0simoo commented Apr 23, 2024

Here's the updated search box!

Screenshot 2024-04-23 at 5 15 17 PM

I'm currently adding the new style rules directly to the element in the HTML, which might be bad practice. I've tried creating a new CSS class for the new changes, but I'm having some trouble getting it to work.

@jimchamp
Copy link
Collaborator

jimchamp commented Apr 23, 2024

Any changes that you make to a less file will need to be transpiled into CSS. You should see your changes after running the following:
docker compose exec web make css

More information about this can be found here.

@0simoo 0simoo linked a pull request Apr 24, 2024 that will close this issue
@0simoo
Copy link

0simoo commented Apr 25, 2024

I've made a pull request with the new designs. Let me know what you think! @jimchamp

@github-actions github-actions bot added the Needs: Response Issues which require feedback from lead label Apr 26, 2024
@mekarpeles mekarpeles added Lead: @cdrini Issues overseen by Drini (Staff: Team Lead & Solr, Library Explorer, i18n) [managed] and removed Lead: @jimchamp Issues overseen by Jim (Front-end Lead, BookNotes) [managed] labels May 3, 2024
@mekarpeles
Copy link
Member

After decision today during design call, we'd like to move forward with a very similar design, inspired by what we already use in the header:

image

Instead of having "Search" be a button to the right of the input, we'll have a magnifying glass be the button within the input area.

Finally, we'd like to re-use this paradigm / component as a template for:

For now, we've decided that this template should be different that the one in the header navigation to keep things simple. cc: @0simoo

Also, just to be clear, we're not asking for all of these changes in 1 PR, for now, this PR can be closed when we have a template that achieves the design in this comment and it's being applied to the various search pages.

@0simoo
Copy link

0simoo commented May 6, 2024

So I should change the designs to the magnifying glass one? I think I'm bit confused about what you mean by the template should be different than the one in the header.

@RayBB
Copy link
Collaborator Author

RayBB commented May 6, 2024

@0simoo what he means is
create: /openlibrary/templates/search/search_bar.html

In that file put (or whatever it is):
<input type="text" class="search-input" name="q" size="100" value="$q"/><input type="submit" class="search-btn" value="$_('Search')"/>

In openlibrary/templates/search/authors.html (and all the other search pages) add:
$:render_template("search/search_bar")

As for the other comment:

should be different than the one in the header

^ This just means don't try to reuse the search bar we currently have here

<form class="search-bar-input" action="/search" method="get">
<input type="text" name="q" placeholder="$_('Search')" aria-label="Search" autocomplete="off">
<input name="mode" type="checkbox" aria-hidden="true" aria-label="Search checkbox" checked="checked" value="" id="ftokenstop" class="hidden instantsearch-mode">
<input type="submit" value="" class="search-bar-submit" aria-label="Search submit">
<div class="vertical-separator"></div>
<a
id="barcode_scanner_link"
class="search-by-barcode-submit"
aria-label="$_('Search by barcode')"
title="$_('Search by barcode')"
href="/barcodescanner?returnTo=/isbn/$$$$$$"
>
</a>
</form>

@0simoo
Copy link

0simoo commented May 8, 2024

I've made a commit to the PR. Let me know if there's anything I misunderstood or should fix!

@mekarpeles mekarpeles removed the Needs: Response Issues which require feedback from lead label May 20, 2024
@RayBB RayBB added the Needs: Staff / Internal Reviewed a PR but don't have merge powers? Use this. label May 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Good First Issue Easy issue. Good for newcomers. [managed] Lead: @cdrini Issues overseen by Drini (Staff: Team Lead & Solr, Library Explorer, i18n) [managed] Needs: Staff / Internal Reviewed a PR but don't have merge powers? Use this. Priority: 3 Issues that we can consider at our leisure. [managed] Type: Feature Request Issue describes a feature or enhancement we'd like to implement. [managed]
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants