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

Update search.html #211

Merged
merged 5 commits into from
Nov 7, 2024
Merged

Update search.html #211

merged 5 commits into from
Nov 7, 2024

Conversation

tdlowden
Copy link
Member

@tdlowden tdlowden commented Aug 29, 2024

I don't like the way this appears. Need some CSS help to get it across the finish line.

Addresses GSA/data.gov#4697

@tdlowden tdlowden requested a review from btylerburton August 29, 2024 19:12
@tdlowden
Copy link
Member Author

tdlowden commented Aug 29, 2024

From playing with this in Dev Tools:

image

I am not savvy enough to make the link underlined and/or a better color, or to move this text closer under the search box. But I think those would both be good additions. Is there a CSS class for regular text we could apply?

@btylerburton
Copy link
Contributor

Added some styling. Let me know what you think...

Screenshot 2024-08-30 at 2 23 20 PM

@tdlowden
Copy link
Member Author

tdlowden commented Sep 3, 2024

I do like the way this looks, thanks! Reflecting on it with fresh eyes, I am not sure my text is the best choice. using "search operators" may be confusing. I will think about plain language text here, but the styling is nice, thanks!

@btylerburton
Copy link
Contributor

Don't see what you're looking for? For more info on how to search, please see "Search in Detail".?

@FuhuXia
Copy link
Member

FuhuXia commented Sep 4, 2024

What does it look like with other UI elements?
image

@tdlowden
Copy link
Member Author

tdlowden commented Sep 4, 2024

Good catch, @FuhuXia. It just disappears eh?

Maybe I don't know enough front-end to have picked up this ticket 😕

@FuhuXia
Copy link
Member

FuhuXia commented Sep 4, 2024

Good catch, @FuhuXia. It just disappears eh?

Maybe I don't know enough front-end to have picked up this ticket 😕

No. the screenshot I took is from prod, not from the changed theme. I just want to make sure the new text you added looks ok with other UI elements in terms of spacing and styling.

We can deploy your change to catalog-dev to see how it looks like with other UI elements.

@btylerburton
Copy link
Contributor

I tested locally against the other error message to: "Please try another search.", which appears when you get no results, and it looked fine, but definitely worth testing that case as well. I can't recall if I did.

@btylerburton
Copy link
Contributor

@tdlowden what's the status on this? you need any help from dev team to get it up on a test environment?

@tdlowden
Copy link
Member Author

tdlowden commented Oct 8, 2024

Yes, we kind of punted until we could get it up on dev to make sure it all worked well. The dev got screwy for a bit. I think we should try to push this to dev and QA

@btylerburton btylerburton marked this pull request as ready for review October 21, 2024 15:50
@btylerburton btylerburton requested a review from a team October 21, 2024 15:50
Copy link
Contributor

@btylerburton btylerburton left a comment

Choose a reason for hiding this comment

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

If this looks good in the UI, let's go for it!

@rshewitt rshewitt merged commit c1a5cf9 into main Nov 7, 2024
4 checks passed
@rshewitt rshewitt deleted the tdlowden-patch-1 branch November 7, 2024 22:43
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.

4 participants