Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

search: Enable search history button by default#44544

Merged
fkling merged 2 commits into
mainfrom
fkling/enable-history-by-default
Nov 17, 2022
Merged

search: Enable search history button by default#44544
fkling merged 2 commits into
mainfrom
fkling/enable-history-by-default

Conversation

@fkling

@fkling fkling commented Nov 17, 2022

Copy link
Copy Markdown
Contributor

This enables the search history by default. Inline history has already been enabled.
This also reverts the change to hide the copy query button on the search homepage.

Test plan

Accessed search home page and results page locally.

App preview:

Check out the client app preview documentation to learn more.

This enables the search history by default. Inline history has already
been enabled.
This also reverts the change to hide the copy query button on the search
homepage.
@fkling fkling added the team/search-product Issues owned by the search product team label Nov 17, 2022
@fkling fkling requested review from a team and quinnkeast November 17, 2022 11:49
@cla-bot cla-bot Bot added the cla-signed label Nov 17, 2022
onChange={props.setQueryState}
onSubmit={onSubmit}
autoFocus={!showSearchHistory && !isTouchOnlyDevice && props.autoFocus !== false}
autoFocus={!isTouchOnlyDevice && props.autoFocus !== false}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I believe this was actually a left over from the first search history experiment. The input should always be focused on the search home page.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yes, it's a leftover, and this change is correct—it should focus on load!

@sourcegraph-bot

sourcegraph-bot commented Nov 17, 2022

Copy link
Copy Markdown
Contributor

Codenotify: Notifying subscribers in CODENOTIFY files for diff ba327d5...b3f127b.

Notify File(s)
@limitedmage client/web/src/search/home/SearchPageInput.tsx
client/web/src/search/input/SearchNavbarItem.tsx

@sg-e2e-regression-test-bob

sg-e2e-regression-test-bob commented Nov 17, 2022

Copy link
Copy Markdown

Bundle size report 📦

Initial size Total size Async size Modules
-0.00% (-0.04 kb) -0.01% (-0.80 kb) -0.01% (-0.76 kb) 0.00% (0)

Look at the Statoscope report for a full comparison between the commits b3f127b and ba327d5 or learn more.

Open explanation
  • Initial size is the size of the initial bundle (the one that is loaded when you open the page)
  • Total size is the size of the initial bundle + all the async loaded chunks
  • Async size is the size of all the async loaded chunks
  • Modules is the number of modules in the initial bundle

applySuggestionsOnEnter={applySuggestionsOnEnter}
showCopyQueryButton={!showSearchHistory}
showSearchHistory={showSearchHistory}
showSearchHistory={true}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should users be able to turn it off? I'm fine if it's permanently enabled. @quinnkeast

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

At least for now, no—I'm of a mind we should avoid introducing additional complexity given we'll likely do larger more holistic changes to the search input.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Great, even better 👍

@fkling fkling merged commit da3deb8 into main Nov 17, 2022
@fkling fkling deleted the fkling/enable-history-by-default branch November 17, 2022 12:16
@fkling fkling changed the title search: Enable serach history button by default search: Enable search history button by default Nov 17, 2022

@limitedmage limitedmage left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice! Can we remove the feature flag from the typings?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

cla-signed team/search-product Issues owned by the search product team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants