Add storybook for HierarchicalLocationsView, remove hardcoded theme colors #14818
Conversation
Codecov Report
@@ Coverage Diff @@
## main #14818 +/- ##
==========================================
+ Coverage 52.19% 52.32% +0.12%
==========================================
Files 1556 1558 +2
Lines 79527 79655 +128
Branches 7089 7127 +38
==========================================
+ Hits 41509 41677 +168
+ Misses 34268 34226 -42
- Partials 3750 3752 +2
|
|
Sorry approved the wrong PR |
client/shared/src/util/url.ts
Outdated
| @@ -169,7 +169,7 @@ const parsePosition = (string: string): Position => { | |||
| */ | |||
| export function parseRepoURI(uri: RepoURI): ParsedRepoURI { | |||
| const parsed = new URL(uri) | |||
| const repoName = parsed.hostname + parsed.pathname | |||
| const repoName = parsed.hostname + parsed.pathname.replace(/^\/\//, '') | |||
There was a problem hiding this comment.
question: Why is this needed? Where do those double slashes come from?
There was a problem hiding this comment.
Ah, I forgot to extract this and deal with it separately. I will remove it from this PR.
In storybooks, there is no URL polyfill, so it treats git:// URIs as having a pathname of //example.com/foo and no hostname. In the main app, our URL polyfill (incorrectly per the spec) treats URIs of all schemes the same, so this is not needed. The reason I included this change here is that the storybook for this component would show the repo name as //github.com/foo/bar, which was weird. But this is a bigger change and should not be in this PR.
BTW, I think it would be good to completely remove these git:// URIs from this part of our app if possible (and probably even everywhere). What do you think?
There was a problem hiding this comment.
We should just include all polyfills in Storybook - we already include e.g. focus-visible. Iirc we have a shared polyfill.ts file that we could just properly share with the webapp.
URL polyfill comes from babel-preset-env/core-js, which we share in Storybook too, so that should work.
BTW, I think it would be good to completely remove these git:// URIs from this part of our app if possible (and probably even everywhere). What do you think?
I totally agree, these URIs are a pain everywhere they exist. I just never had the time to really rip them out. I think we're also gonna add alternative APIs to extensions next iteration so URI parsing is no longer needed, but even just not relying on them outside of the extension host would be great.
No description provided.