(feat) add repo metadata editing UI

[erzhtor]

Part of https://github.com/sourcegraph/pr-faqs/issues/96.

Currently, repo metadata can be added directly only via GQL API or src cli. The PR:

• update repo metadata display UI on search results page and repo root pages according to new figma designs
• adds repo metadata editing UI for site admins to remove/add metadata.

NOTE: As a follow-up we will adding RBAC for not only site admins to have editing access to it.

Test plan

• sg start
• Enable the repository-metadata feature flag
• Add some key-value pairs through API (currently no UI for adding) GQL mutation addRepoKeyValuePair API
• Search for that repo > go to repo root page

Screenshots

Screen.Recording.2023-04-25.at.23.14.35.mov

[sourcegraph-bot]

Codenotify: Notifying subscribers in CODENOTIFY files for diff 08c0bf1...160c42c.

Notify	File(s)
@fkling	client/branded/src/search-ui/components/RepoMetadata.module.scss client/branded/src/search-ui/components/RepoMetadata.story.tsx client/branded/src/search-ui/components/RepoMetadata.tsx client/branded/src/search-ui/components/RepoSearchResult.tsx
@limitedmage	client/branded/src/search-ui/components/RepoMetadata.module.scss client/branded/src/search-ui/components/RepoMetadata.story.tsx client/branded/src/search-ui/components/RepoMetadata.tsx client/branded/src/search-ui/components/RepoSearchResult.tsx

[limitedmage]

Very cool! Left some comments/questions.

[vdavid]

Before merging a change that introduces so many UI elements, I'd definitely ask for a design review to ensure that my additions match the rest of the product.

My incompetent observations from a UX and UI design POV:

• The UI looks a bit plain—I'll probably add a short paragraph of explanation about what happens here, with a docs link for
  more info on repo metadata. I saw there are examples
• The modal also feels somehow empty/plain (maybe some heading would help) and somehow a bit off in terms of design.
• Also, the "Add new metadata" button label sounds a bit off to me because "metadata" is not countable. Maybe "Add metadata field"? But a native English speaker might have a better idea.
• Have you tested the UI from an accessibility POV, like, is it possible & relatively comfortable to use with the keyboard only?

[vdavid]

Also, a short Loom demo would help me see this in action and better understand what it does. (If you want to get a round of design review, then I'd probably demo it after the changes to show the polished version.)

[vdavid]

Oh, I just saw you tagged Rob as a reviewer. I was only looking at the tags, looking for design-qa or similar. In any case, I'd like to wait for the design review first because I imagine there might be significant changes in the code after it. Feel free to ping me again for a re-review!

[toolmantim]

Thanks! Great to see it come to life.

But I'll echo what @vdavid said — this needs a fair amount of design changes, which is going to be hard to do in a PR review.

Process wise, let's make sure people know they have someone on the design team to collab with earlier. Also love the idea of a qa-design team in GH, though I wouldn't replicate this process again.

This PR might have to stay parked until I've had time to do the designs and a chance to get it all working locally end-to-end (+1 for Looms too!).

I'm starting on the designs, and getting it working locally, but it's my first week so will take a little bit longer than usual 🙏🏼

[vdavid]

Thanks for sharing your take, @toolmantim!

To be fair, I think the PR review is a good place to do this. I've seen it done many times this way and I usually found that it helped the designer to see a working version of the feature, and then iterating on the design worked out well and was usually fairly quick. (I wouldn't expect more than a few days in this case.)

Btw we already have the "design-qa" tag, just we also have a few more, and I never know which one to use :)

[toolmantim]

Thanks. Yeah, I'd agree on smaller stuff or draft PRs. And nothing beats working software! But as soon as it hits multiple screens and many related PRs then some other types of collabs earlier can be helpful from my side.

But here is good for now! Stay tuned…

[erzhtor]

@juliasourceress, @toolmantim, @vdavid I've updated the PR according to new designs, and would appreciate your reviews.

[toolmantim]

Thanks @erzhtor! I just pushed a few design changes around wording, color and spacing.

A few small things, that are totally non-blockers:

• It might be nice to re-focus the "Key" input after successfully adding a key
• The error message for duplicates is a raw pg validation error message — not sure if intentional, but perhaps could be handled nicely?
• I spotted you can't add multiple key-value pairs with the same key but different values, which was a bit unexpected. But perhaps that's by design?

[sourcegraph-bot]

Codenotify: Notifying subscribers in OWNERS files for diff 08c0bf1...160c42c.

Notify	File(s)
@sourcegraph/code-exploration-devs	client/wildcard/src/global-styles/colors.scss
@vovakulikov	client/wildcard/src/global-styles/colors.scss

[sg-e2e-regression-test-bob]

Bundle size report 📦

Initial size	Total size	Async size	Modules
0.01% (+0.37 kb)	0.09% (+13.18 kb) 🔺	0.11% (+12.81 kb) 🔺	0.13% (+1) 🔺

Look at the Statoscope report for a full comparison between the commits 160c42c and d8f53d2 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

[erzhtor]

Thanks, @toolmantim for the review and feedback.

• The error message for duplicates is a raw pg validation error message — not sure if intentional, but perhaps could be handled nicely?

I have todo on my list regarding nicer error messages for crud operations, so will handle them separately.

• I spotted you can't add multiple key-value pairs with the same key but different values, which was a bit unexpected. But perhaps that's by design?

I asked the same question initially, but turned it was made intentionally by design, here is a slack discussion. TLDR nobody from customers who are already using did ask this feature, so we decided to keep as is until getting an explicit request.