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

Setup UI: Add remote repositories setup step UI#47572

Merged
vovakulikov merged 22 commits intomainfrom
vk/add-remote-repositories-setup-ui
Feb 20, 2023
Merged

Setup UI: Add remote repositories setup step UI#47572
vovakulikov merged 22 commits intomainfrom
vk/add-remote-repositories-setup-ui

Conversation

@vovakulikov
Copy link
Contributor

@vovakulikov vovakulikov commented Feb 13, 2023

Part of https://github.com/sourcegraph/sourcegraph/issues/47237

This PR adds a first iteration of UI implementation for the code host connection setup step. It does use some existing API, but it isn't connected yet to the specific API about the setup wizard (it will be addressed in a separate step)

I recorder a quick loom with an overview on the current progress https://www.loom.com/share/19f8179b910d421fa1dc283d2b86b94f

Todo

  • Implement common UI
  • Remove enterprise-like imports
  • Make setup wizard lazy loaded

Further steps

  • Connect this UI with external serives creation/updating mutations
  • Implement delete functionality (connect delete buttons to delete mutation)
  • Connect next button availability state to remote repositories step state (make it available only when you have at least one connected code host)
  • Implement code host progress bar (info block next to footer layout)

Test plan

  • N/A

App preview:

Check out the client app preview documentation to learn more.

@vovakulikov vovakulikov self-assigned this Feb 13, 2023
@cla-bot cla-bot bot added the cla-signed label Feb 13, 2023
vovakulikov referenced this pull request Feb 15, 2023
Part of https://github.com/sourcegraph/sourcegraph/issues/47237
Preparation for https://github.com/sourcegraph/sourcegraph/pull/47572

This PR adds a new type of file hook - useControlledField. There are a
few cases when we want to store the field's value, not in the form but
somewhere else (like in another field store or some external store)
useControlledField doesn't bind with useForm it only adds a validation
pipeline to the data; that the consumer is supposed to provide.

Also, this PR fixes one small visual problem with insight creation UI (I
didn't want to open yet another one-line PR for it)
vovakulikov referenced this pull request Feb 15, 2023
Preparation for https://github.com/sourcegraph/sourcegraph/pull/47572

In the https://github.com/sourcegraph/sourcegraph/pull/47572 PR, we use
some form logic and UI component currently placed in the insights
directory (since originally, we came up with this logic for the code
insights creation UI, it's still placed there).

Since in https://github.com/sourcegraph/sourcegraph/pull/47572 we need
to import something enterprise (insights) in something OSS (setup
wizard), we have import validation error. It's a forbidden practice to
have enterprise in OSS. So, this PR just moves insight form validation
logic and UI to the wildcard package.
@vovakulikov vovakulikov force-pushed the vk/add-remote-repositories-setup-ui branch from a71b8f8 to f4d0e27 Compare February 16, 2023 23:25
@sg-e2e-regression-test-bob
Copy link

sg-e2e-regression-test-bob commented Feb 16, 2023

Bundle size report 📦

Initial size Total size Async size Modules
-0.07% (-2.06 kb) 0.24% (+35.81 kb) 🔺 0.32% (+37.88 kb) 🔺 1.05% (+8) 🔺

Look at the Statoscope report for a full comparison between the commits a550dcc and 38aa664 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

@vovakulikov vovakulikov marked this pull request as ready for review February 20, 2023 14:29
Copy link
Contributor

@st0nebreaker st0nebreaker left a comment

Choose a reason for hiding this comment

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

Looks really good! The layered routing logic is awesome.

Just a few minor comments below. Also re: UX Flow, more of a design nit, I think the left navigation panel should have an Add more code hosts button, because it wouldn't be clear to the user that canceling edit mode leads them to the add more code hosts view. WDYT?

Screenshot 2023-02-20 at 10 41 54 AM

Comment on lines +267 to +276
<RadioGroupSection
name="repositories"
value="repositories"
checked={isSetRepositories}
labelId="repos"
label="Add selected repositories"
onChange={handleRepositoriesModeChange}
>
<GithubRepositoriesPicker repositories={repositories} onChange={handleRepositoriesChange} />
</RadioGroupSection>
Copy link
Contributor

Choose a reason for hiding this comment

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

One more thing. I'm noticing values here aren't persisting when the top parent is clicked/unclicked. Will this persist after we connect the suggestions to the new api? (Video below of behavior)

Screen.Recording.2023-02-20.at.1.09.54.PM.mov

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm. I see this is interesting. At the moment in works that, all edits that we're doing edit the json configuration, so unchecked checkbox for repos means that we don't have this field in the JSON configuration, than you check this checkbox, and we add an empty field in the JSON configuration, if pick some repositories we add them in this field in JSON.

When you unchecked checkbox, we remove this field from JSON, that means that we remove all checked repositories, in theory we can add presave them somewhere outside of JSON so when you click check (checked it) we will populate JSON configuration with these saved values. I think it may be a good follow up.

Copy link
Contributor

Choose a reason for hiding this comment

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

I see, thanks for the explanation. Yea a different enhancement could even be saving the edit state somewhere & making sure the edit state repos are at least the top suggested repos under the input.

…p/components/code-hosts/github/GithubConnectView.module.scss

Co-authored-by: Becca Steinbrecher <beccasteinbrecher@gmail.com>
@sourcegraph-bot
Copy link
Contributor

sourcegraph-bot commented Feb 20, 2023

Codenotify: Notifying subscribers in CODENOTIFY files for diff 6ff96b3...a550dcc.

Notify File(s)
@efritz client/web/src/enterprise/codeintel/configuration/components/RepositoryPatternList.module.scss

@sourcegraph-bot
Copy link
Contributor

sourcegraph-bot commented Feb 20, 2023

Codenotify: Notifying subscribers in OWNERS files for diff 6ff96b3...a550dcc.

Notify File(s)
@sourcegraph/code-exploration-devs client/wildcard/src/components/Combobox/Combobox.module.scss
client/wildcard/src/components/Combobox/Combobox.tsx
client/wildcard/src/components/Container/Container.tsx
client/wildcard/src/components/Tabs/useShouldPanelRender.ts
client/wildcard/src/components/Tooltip/Tooltip.tsx
client/wildcard/src/hooks/useDebounce.ts

@vovakulikov
Copy link
Contributor Author

Thanks, @st0nebraker, for your review. I addressed UI suggestions and comments; I'm about to merge it now. 😉

@vovakulikov vovakulikov merged commit fd4fd0e into main Feb 20, 2023
@vovakulikov vovakulikov deleted the vk/add-remote-repositories-setup-ui branch February 20, 2023 22:59
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants