-
Notifications
You must be signed in to change notification settings - Fork 73
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
Browser Extension 2.0 #81
Conversation
7e87346
to
d9a1df2
Compare
d5d0b2d
to
d5329b8
Compare
d5329b8
to
7b45e11
Compare
@filiptronicek and @selfcontained I have fixed the issues we've seen yesterday. 👍 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot for taking this on, @svenefftinge. It lays a solid foundation upon which we can continue to build v2.
We also seem to be getting manifest version 3 support for free with this (could close #65) as well as an easier way to publish to Safari.
I've only tested on Chrome, because both of my Firefox installs kept saying that the packaged extension was corrupt, but I'm guessing that's something wrong on my end.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for a follow-up: make this a nightly GitHub Action workflow, also running on PRs as a check
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
</div> | ||
</div> | ||
|
||
{showDropdown && ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in a follow-up: let's make this accessible so you can open the dropdown with the keyboard
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return ( | ||
<div id="gitpod-btn-nav" title="Gitpod" className={classNames("gitpod-button", application, ...(additionalClassNames || []))}> | ||
<div className={classNames("button")}> | ||
<a className={classNames("button-part", "action")} href={actions[0].href}> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Personally, I miss the automatic opening in a new tab. If changing this is intended, I'll be happy to contribute something to the settings later to optionally change it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<div className={classNames("button")}> | ||
<a className={classNames("button-part", "action")} href={actions[0].href}> | ||
<span className={classNames("action-label")}> | ||
<Logo className={classNames("action-logo")} width={14} height={14}/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a big fan of the logo here. It feels fresh and makes the button even more easily spotted.
In some contexts (like the file viewer on GitHub), we could try experimenting with the omission of the text and just having the logo there. Just an idea, but could be nice (most likely adds unnecessary complexity at this point)
Before | After |
---|---|
const updateAddress = useCallback((address: string) => { | ||
address = address.trim(); | ||
address = address.endsWith("/") ? address.slice(0, -1) : address; | ||
const err = validate(address); | ||
setError(err); | ||
setAddress(address); | ||
}, [setAddress, setError]); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in a follow up, I'll try to add the Desktop App parser here, so more formats are accepted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is really cool, love it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could look into how this looks in all of the different GitLab themes, but from what I've tested, it looks great everywhere even in dark mode.
Thank you @filiptronicek 🙏 I'll merge this so we can add some of the things you mentioned in smaller follow-ups. |
This is a rewrite of the browser extension using the plasma framework, which allows to use React components and has a couple of other advantages.
The extension supports
installations on any domain.
It also now starts workspaces with
autostart=true
which will automatically forward when landing in gitpod's create workspace page. A secondary button to "Open with options" is available in case a user wants to select a different IDE/ws class.The rewrite also includes tests for the element selectors used to check if and where a button should be installed, so we get noticed when a website changes its design and breaks our integration.