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

Circulars: Lucene Shortcuts and Archive Page Documentation #2589

Closed
wants to merge 12 commits into from
Closed

Circulars: Lucene Shortcuts and Archive Page Documentation #2589

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
0d85fbb
initial commit
tylerbarna Sep 18, 2024
1d68e5c
hide lucene menu behind feature flag
tylerbarna Sep 18, 2024
08929d3
add basic documentation in accordion menu
tylerbarna Sep 18, 2024
1537a18
simplify accordion
tylerbarna Sep 25, 2024
c52f390
initial structure for quick actions
tylerbarna Sep 25, 2024
fafbb38
change variable name
tylerbarna Sep 25, 2024
40e8791
added structure for buttons to update query
tylerbarna Sep 25, 2024
575a452
correct feature flag typo
tylerbarna Sep 25, 2024
e965246
add quick action buttons
tylerbarna Sep 25, 2024
6401a2d
fix typo
tylerbarna Sep 25, 2024
2e6567d
minor style changes
tylerbarna Sep 25, 2024
71f10d4
address changes
tylerbarna Oct 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 53 additions & 0 deletions app/routes/circulars._archive._index/CircularsLuceneMenu.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
import { Link } from '@remix-run/react'
import { Button } from '@trussworks/react-uswds'

Check warning on line 2 in app/routes/circulars._archive._index/CircularsLuceneMenu.tsx

View check run for this annotation

Codecov / codecov/patch

app/routes/circulars._archive._index/CircularsLuceneMenu.tsx#L1-L2 

Added lines #L1 - L2 were not covered by tests

export function LuceneAccordion({

Check warning on line 4 in app/routes/circulars._archive._index/CircularsLuceneMenu.tsx

View check run for this annotation

Codecov / codecov/patch

app/routes/circulars._archive._index/CircularsLuceneMenu.tsx#L4 

Added line #L4 was not covered by tests
query,
querySetter,
}: {
query?: string
querySetter: (value: string) => void
}) {
function populateSearch(value: string) {
querySetter(`${query} ${value}`)

Check warning on line 12 in app/routes/circulars._archive._index/CircularsLuceneMenu.tsx

View check run for this annotation

Codecov / codecov/patch

app/routes/circulars._archive._index/CircularsLuceneMenu.tsx#L11-L12 

Added lines #L11 - L12 were not covered by tests
}
return (

Check warning on line 14 in app/routes/circulars._archive._index/CircularsLuceneMenu.tsx

View check run for this annotation

Codecov / codecov/patch

app/routes/circulars._archive._index/CircularsLuceneMenu.tsx#L14 

Added line #L14 was not covered by tests
<details>
<summary>Advanced Search</summary>

<div>
To narrow the search results, use Lucene search syntax. This allows for
specifying which circular field to search (submitter, subject, and/or
body). Further documentation can be found on the{' '}
<Link className="usa-link" to="/docs/circulars/lucene">
Lucene Search Syntax Page
</Link>
{'. '}
<h4>Lucene Examples:</h4>
<div>
<Button
type="button"
outline
onClick={() => populateSearch('subject:"Swift"')}

Check warning on line 31 in app/routes/circulars._archive._index/CircularsLuceneMenu.tsx

View check run for this annotation

Codecov / codecov/patch

app/routes/circulars._archive._index/CircularsLuceneMenu.tsx#L31 

Added line #L31 was not covered by tests
>
subject:"Swift"
</Button>
<Button
type="button"
outline
onClick={() => populateSearch('body:"GRB"')}

Check warning on line 38 in app/routes/circulars._archive._index/CircularsLuceneMenu.tsx

View check run for this annotation

Codecov / codecov/patch

app/routes/circulars._archive._index/CircularsLuceneMenu.tsx#L38 

Added line #L38 was not covered by tests
>
body:"GRB"
</Button>
<Button
type="button"
outline
onClick={() => populateSearch('submitter:"Tomas Ahumada Mena"')}
>

Check warning on line 46 in app/routes/circulars._archive._index/CircularsLuceneMenu.tsx

View check run for this annotation

Codecov / codecov/patch

app/routes/circulars._archive._index/CircularsLuceneMenu.tsx#L45-L46 

Added lines #L45 - L46 were not covered by tests
submitter:"Tomas Ahumada Mena"
</Button>
</div>
</div>
</details>
)
}
7 changes: 6 additions & 1 deletion app/routes/circulars._archive._index/route.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,14 +43,15 @@
import CircularPagination from './CircularPagination'
import CircularsHeader from './CircularsHeader'
import CircularsIndex from './CircularsIndex'
import { LuceneAccordion } from './CircularsLuceneMenu'
import { DateSelector } from './DateSelectorMenu'
import { SortSelector } from './SortSelectorButton'
import Hint from '~/components/Hint'
import { ToolbarButtonGroup } from '~/components/ToolbarButtonGroup'
import { origin } from '~/lib/env.server'
import { getFormDataString } from '~/lib/utils'
import { postZendeskRequest } from '~/lib/zendesk.server'
import { useModStatus } from '~/root'
import { useFeature, useModStatus } from '~/root'

Check warning on line 54 in app/routes/circulars._archive._index/route.tsx

View check run for this annotation

Codecov / codecov/patch

app/routes/circulars._archive._index/route.tsx#L54 

Added line #L54 was not covered by tests

import searchImg from 'nasawds/src/img/usa-icons-bg/search--white.svg'

Expand Down Expand Up @@ -227,6 +228,7 @@
name="query"
type="search"
defaultValue={inputQuery}
value={inputQuery}
Copy link
Member

Choose a reason for hiding this comment

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

I don't remember where I read this, but I think it's considered a common mistake to have an input that, on change, updates a state variable, and whose value comes from a state variable. @dakota002, @Courey, have you heard that? Or am I misremembering?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure on that, I think it might be dependent on where in the general hierarchy of components those state values are used. Like if they need to affect a value in a child/parent component. And if they are basic inputs vs more complex components. But I think generally speaking your idea sounds correct

I think there was something going on here where, when the page reloads due to a form submission, the value was not updating correctly, like the defaultValue would still show the previous value, even though the query string would update. @tylerbarna is this correct or am I thinking of another thing?

Copy link
Member Author

@tylerbarna tylerbarna Oct 8, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

@dakota002 without setting value the search bar won't update to reflect the current query string inside the search bar until the page is reloaded, so there's not visual feedback that one of the quick action buttons have been pressed until you reload the page; we don't want the page to reload every time a quick action button is pressed since the contents of the quick action button is likely not the exact term users are looking to search for

Copy link
Member

Choose a reason for hiding this comment

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

Sorry that this has been holding things up. I am still not sure if this is the right way to programmatically modify the search field. To unblock this, let's just not automatically fill in the search field right now.

Copy link
Member Author

Choose a reason for hiding this comment

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

@lpsinger apologies for missing this last week, I'm going to close this PR and open a new one with just the in-page documentation

placeholder="Search"
aria-describedby="searchHint"
onChange={({ target: { form, value } }) => {
Expand Down Expand Up @@ -260,6 +262,9 @@
To navigate to a specific circular, enter the associated Circular ID
(e.g. 'gcn123', 'Circular 123', or '123').
</Hint>
{useFeature('CIRCULARS_LUCENE') && (
<LuceneAccordion query={inputQuery} querySetter={setInputQuery} />
)}
{clean && (
<>
<CircularsIndex
Expand Down
2 changes: 1 addition & 1 deletion app/routes/circulars/circulars.server.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ export async function search({
}

const queryObj = query
? feature('CICRULARS_LUCENE')
? feature('CIRCULARS_LUCENE')
? {
simple_query_string: {
query,
Expand Down
Loading