Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

feat(extensions): add helper header above extensions search #2501

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jsjoeio merged 2 commits into master from issue-2132-help-info-extension-search-view
Jan 14, 2021

Conversation

Copy link
Contributor

@jsjoeio jsjoeio commented Dec 21, 2020 •
edited
Loading

Add a short message above the search box on the Extensions panel. This helps explain the extension divergence to the user.

If they click dismiss, it stores an item in localStorage to prevent the message from showing up on subsequent loads.

Builds off the work previously done by @cmoog in #2185

Screenshots

2020年12月21日 10 12 37

And it uses the correct colors based on theme:

IMG_0042.MP4

Checklist

cmoog reacted with eyes emoji
@jsjoeio jsjoeio self-assigned this Dec 21, 2020
@jsjoeio jsjoeio changed the title (削除) feat (extensions): add helper header above extensions search (削除ここまで) (追記) feat(extensions): add helper header above extensions search (追記ここまで) Dec 21, 2020
Copy link
Member

@code-asher code-asher left a comment

Choose a reason for hiding this comment

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

Looks great!

jsjoeio reacted with heart emoji
Copy link
Contributor

nhooyr commented Jan 5, 2021

Love the gif btw 🔥
jsjoeio reacted with heart emoji

@jsjoeio jsjoeio force-pushed the issue-2132-help-info-extension-search-view branch 2 times, most recently from dc2445c to 7060f60 Compare January 7, 2021 18:25
@jsjoeio jsjoeio requested a review from nhooyr January 7, 2021 18:28
Copy link
Contributor Author

jsjoeio commented Jan 7, 2021

rsync: change_dir "/github/workspace//typings/pluginapi.d.tsrelease" failed: No such file or directory (2)
2057
rsync error: some files/attrs were not transferred (see previous errors) (code 23) at main.c(1207) [sender=3.1.3]

I’m not totally sure why the release step is failing. Any ideas?

Copy link
Member

code-asher commented Jan 8, 2021 •
edited
Loading

Ahh, sorry about the failed release step, it's my fault. I pushed a fix to master, please feel free to rebase.
jsjoeio reacted with thumbs up emoji

@jsjoeio jsjoeio force-pushed the issue-2132-help-info-extension-search-view branch from 044d992 to 93ca456 Compare January 8, 2021 17:51
Copy link
Contributor Author

jsjoeio commented Jan 11, 2021

Bump @nhooyr - ready for a second review

Copy link
Contributor

nhooyr commented Jan 11, 2021

Can confirm things work for me!

But the links appear to be styled very hard to read for some reason?

Are we maybe missing a class on the links or some styles?

Screen Shot 2021年01月11日 at 12 43 38 PM

Copy link
Contributor

nhooyr commented Jan 11, 2021

Like there's little contrast compared to the other links like:
Screen Shot 2021年01月11日 at 12 44 42 PM

Copy link
Contributor

nhooyr commented Jan 11, 2021

@jsjoeio make sure to squash the really long Update file commits generated by github.

Copy link
Contributor Author

jsjoeio commented Jan 11, 2021

Can confirm things work for me!

But the links appear to be styled very hard to read for some reason?

Are we maybe missing a class on the links or some styles?

Hmm! Good catch. Let me look into it.

Copy link
Contributor Author

jsjoeio commented Jan 11, 2021

@jsjoeio make sure to squash the really long Update file commits generated by github.

You got it!

Copy link
Contributor Author

jsjoeio commented Jan 11, 2021

Like there's little contrast compared to the other links like:

I’m having trouble figuring this out on iPad since I don’t have access to DevTools to inspect the styles 🤔 I’m looking at the Welcome page and don’t see anything jumping out at me. The links in the list have no classes so i’m not sure where the link color is coming from.

4FDD2F99-E546-4194-8536-85CDC6745F09

The easiest solution would be to hard-code the link style to match the rest of code-server but I worry that wouldn’t work well across various themes. I’ll keep investigating.

Copy link
Contributor Author

jsjoeio commented Jan 11, 2021 •
edited
Loading

Well...hard-coding the color works. Not sure what else to try. Any thoughts/suggestions @nhooyr ?

8B5C8CEE-B40C-446B-A4CD-271AE6B82FF5

@jsjoeio jsjoeio force-pushed the issue-2132-help-info-extension-search-view branch from 93ca456 to 3af5c25 Compare January 11, 2021 23:46
Copy link
Contributor

@nhooyr nhooyr left a comment

Choose a reason for hiding this comment

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

Happy to merge as is but one downside of hard coding the link color like we have is that the :visited style doesn't take any affect and so the links don't turn purple anymore which sucks.

I understand @code-asher has some ideas to fix this?

Copy link
Contributor

nhooyr commented Jan 12, 2021

Or one more I should say, like you said themes will also be a problem. So we can't merge as is then.

Copy link
Contributor Author

jsjoeio commented Jan 12, 2021

Yeah exactly. I'll move this back to draft state then re-request a review when it's ready!

@jsjoeio jsjoeio marked this pull request as draft January 12, 2021 17:14
Copy link
Contributor Author

jsjoeio commented Jan 12, 2021

Fixed the color issues (H/T to @code-asher for pointing me in the right direction)

BEFORE
D01570C0-87A7-47D6-8CC8-7017963CEBA4

AFTER
E59F1722-7B11-4F6B-9A1A-F4518C55B4AE

nhooyr reacted with heart emoji

@jsjoeio jsjoeio marked this pull request as ready for review January 12, 2021 17:43
@jsjoeio jsjoeio requested a review from nhooyr January 12, 2021 17:43
Copy link
Contributor

@nhooyr nhooyr left a comment

Choose a reason for hiding this comment

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

perfecto!! 🔥

I noticed that :visited isn't supported by any of VS Code's other UI links so it's fine that we don't here too.

jsjoeio reacted with rocket emoji
Copy link
Contributor Author

jsjoeio commented Jan 12, 2021

Oh didn't even think about that - thanks for checking! Let's get this bad boy merged in!

Copy link
Contributor Author

jsjoeio commented Jan 12, 2021

After i fix the lint issues :)

@jsjoeio jsjoeio force-pushed the issue-2132-help-info-extension-search-view branch 2 times, most recently from 720a268 to 9d67fa1 Compare January 12, 2021 19:39
jsjoeio and others added 2 commits January 14, 2021 22:40 
Add a short message above the search box on the Extensions panel. This
helps explain the extension divergence to the user.
If they click dismiss, it stores an item in localStorage to prevent the
message from showing up on subsequent loads.
Co-authored-by: Asher <ash@coder.com>
@jsjoeio jsjoeio force-pushed the issue-2132-help-info-extension-search-view branch from 9d67fa1 to 500ba92 Compare January 14, 2021 22:41
@jsjoeio jsjoeio merged commit 3394ece into master Jan 14, 2021
@jsjoeio jsjoeio deleted the issue-2132-help-info-extension-search-view branch January 14, 2021 23:30
@nhooyr nhooyr added this to the v3.8.1 milestone Jan 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@code-asher code-asher code-asher approved these changes

+1 more reviewer

@nhooyr nhooyr nhooyr approved these changes

Reviewers whose approvals may not affect merge requirements
Labels
None yet
Projects
None yet
Milestone
v3.8.1
Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /