Skip to content

feat(webui): add global search access across pages#1595

Open
Pranav-0440 wants to merge 8 commits intoeclipse:masterfrom
Pranav-0440:feat/webui-global-search
Open

feat(webui): add global search access across pages#1595
Pranav-0440 wants to merge 8 commits intoeclipse:masterfrom
Pranav-0440:feat/webui-global-search

Conversation

@Pranav-0440
Copy link
Contributor

fixes : #299

Signed-off-by: Pranav Ghorpade <pranavghorpade61@gmail.com>
Copilot AI review requested due to automatic review settings February 7, 2026 07:43
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR aims to address issue #299 by making search accessible from more pages in the Web UI (so users can start a new extension search without navigating back to the home page).

Changes:

  • Wrap Main route content with a DefaultApp component (intended as a global layout wrapper).
  • Add a search UI (SearchBox) to both default and mobile header menu contents, gated by pageSettings.showSearch.
  • Force-enable the new showSearch flag in the default webui entrypoint page settings.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.

File Description
webui/src/main.tsx Wrapes page content with DefaultApp (currently references a non-existent export).
webui/src/default/menu-content.tsx Adds SearchBox into menu/header content for desktop and mobile, behind pageSettings.showSearch.
webui/src/default/default-app.tsx Augments page settings with showSearch: true at the entrypoint.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 127 to 132
<DefaultApp
pageSettings={props.pageSettings}
service={props.service}
>
{renderPageContent()}
</DefaultApp>
Copy link

Copilot AI Feb 7, 2026

Choose a reason for hiding this comment

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

./default/default-app.tsx is the webui entrypoint that creates the React root and does not export a DefaultApp component. Importing it here and rendering <DefaultApp ...> will fail to compile (and would also introduce an entrypoint/layout coupling). Consider removing this wrapper from Main, or extracting a real layout component (exported from a dedicated module) if you need to wrap all routes.

Copilot uses AI. Check for mistakes.
Comment on lines 32 to 33
import { SearchBox } from '../components/search/search-box';

Copy link

Copilot AI Feb 7, 2026

Choose a reason for hiding this comment

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

SearchBox is imported from ../components/search/search-box, but there is no components/search directory in the repo. This will fail the build; either add the new component/file in this PR or change the import to an existing search component (e.g. reuse ExtensionListSearchfield via a wrapper).

Suggested change
import { SearchBox } from '../components/search/search-box';
type SearchBoxProps = React.InputHTMLAttributes<HTMLInputElement>;
export const SearchBox: FunctionComponent<SearchBoxProps> = (props) => (
<input type='search' {...props} />
);

Copilot uses AI. Check for mistakes.
Comment on lines 122 to 129
const { user, loginProviders, pageSettings } = useContext(MainContext);

return <>
{pageSettings.showSearch && (
<MobileMenuItem>
<SearchBox />
</MobileMenuItem>
)}
Copy link

Copilot AI Feb 7, 2026

Choose a reason for hiding this comment

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

pageSettings is typed as PageSettings via MainContext, but PageSettings currently has no showSearch property. Accessing pageSettings.showSearch will be a TypeScript error. Add showSearch?: boolean to PageSettings (and set it in the page settings factory) or remove the flag and render the search UI unconditionally.

Suggested change
const { user, loginProviders, pageSettings } = useContext(MainContext);
return <>
{pageSettings.showSearch && (
<MobileMenuItem>
<SearchBox />
</MobileMenuItem>
)}
const { user, loginProviders } = useContext(MainContext);
return <>
<MobileMenuItem>
<SearchBox />
</MobileMenuItem>

Copilot uses AI. Check for mistakes.
Comment on lines 219 to 221
{pageSettings.showSearch && (
<SearchBox />
)}
Copy link

Copilot AI Feb 7, 2026

Choose a reason for hiding this comment

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

Same PageSettings.showSearch type issue as above: PageSettings does not define showSearch, so this will not type-check until the interface is updated (or the flag is removed).

Copilot uses AI. Check for mistakes.
{pageSettings.showSearch && (
<MobileMenuItem>
<SearchBox />
</MobileMenuItem>
Copy link

Copilot AI Feb 7, 2026

Choose a reason for hiding this comment

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

There is trailing whitespace after </MobileMenuItem> which may fail lint/format checks. Please remove the extra spaces.

Suggested change
</MobileMenuItem>
</MobileMenuItem>

Copilot uses AI. Check for mistakes.
Comment on lines 58 to 62
const pageSettings = {
...createPageSettings(prefersDarkMode, service.serverUrl, getServerVersion()),
showSearch: true
};

Copy link

Copilot AI Feb 7, 2026

Choose a reason for hiding this comment

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

showSearch: true is being added ad-hoc here rather than being part of the PageSettings contract returned by createPageSettings. If showSearch is intended to be a supported setting, add it to the PageSettings interface and populate it inside createPageSettings so all consumers can rely on the same typed shape.

Suggested change
const pageSettings = {
...createPageSettings(prefersDarkMode, service.serverUrl, getServerVersion()),
showSearch: true
};
const pageSettings = createPageSettings(prefersDarkMode, service.serverUrl, getServerVersion());

Copilot uses AI. Check for mistakes.
@netomi
Copy link
Contributor

netomi commented Feb 7, 2026

thanks for your contribution, please sign the ECA as outlined here https://api.eclipse.org/git/eca/status/gh/eclipse/openvsx/1595.

@Pranav-0440
Copy link
Contributor Author

thanks for your contribution, please sign the ECA as outlined here https://api.eclipse.org/git/eca/status/gh/eclipse/openvsx/1595.

{FD5051CF-F651-422C-86E2-FA33E24D510C}

This problem occurs when signing in.

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.

[ux] Add a search box to every page

2 participants