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

SCC-383x/vqa #38

Merged
merged 7 commits into from
Nov 3, 2023
Merged

SCC-383x/vqa #38

merged 7 commits into from
Nov 3, 2023

Conversation

charmingduchess
Copy link
Contributor

@charmingduchess charmingduchess commented Oct 26, 2023 •
edited
Loading

This combines work called out in 3 VQA tickets (SCC-3830, 3831, and 3832), including:

  • Page title change to “Research Catalog | NYPL”.
  • Add Article Search link
  • SubNav wrapping keeping link text together
    • Note this one i did not follow the spec 100%, but instead implemented a simpler fix... TBD if it is acceptable.
  • Placeholder text in the text input field italicized.
  • An ‘X’ clear button should appear in the text input field once the patron starts typing - I believe this functionality exists in the DS search bar, but let me know if that isn’t the case.
    • this and previous was fixed with my search form update to include the DS search bar
  • 16px margin for Advanced Search button

@vercel
Copy link

vercel bot commented Oct 26, 2023 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
research-catalog ✅ Ready (Inspect) Visit Preview 💬 Add feedback Nov 3, 2023 2:46pm

@charmingduchess charmingduchess changed the title Scc 383 x/vqa SCC-383x/vqa Oct 27, 2023
EdwinGuzman
EdwinGuzman requested changes Oct 27, 2023
View reviewed changes
Copy link
Member

@EdwinGuzman EdwinGuzman left a comment

Choose a reason for hiding this comment

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

Just one update needed to remove the search value from the internal state.

src/components/SearchForm/SearchForm.tsx Show resolved Hide resolved
dgcohen
dgcohen approved these changes Oct 31, 2023
View reviewed changes
Copy link
Contributor

@dgcohen dgcohen left a comment

Choose a reason for hiding this comment

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

Agree with Edwin's comment, but otherwise looks good to me.

@charmingduchess charmingduchess merged commit acac06c into main Nov 3, 2023
2 checks passed
@dgcohen dgcohen deleted the SCC-383X/vqa branch February 1, 2024 16:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@apoorvakhanolkar apoorvakhanolkar apoorvakhanolkar approved these changes

@dgcohen dgcohen dgcohen approved these changes

@EdwinGuzman EdwinGuzman EdwinGuzman approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@charmingduchess @EdwinGuzman @dgcohen @apoorvakhanolkar