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

Fix CSS injection of Phabricator native extension#43868

Merged
philipp-spiess merged 4 commits into
mainfrom
ps/fix-phabricator-native-extension
Nov 3, 2022
Merged

Fix CSS injection of Phabricator native extension#43868
philipp-spiess merged 4 commits into
mainfrom
ps/fix-phabricator-native-extension

Conversation

@philipp-spiess

@philipp-spiess philipp-spiess commented Nov 3, 2022

Copy link
Copy Markdown
Contributor

Fixes #42555

The Phabricator native extension injected the same CSS file twice instead of injecting both style.bundle.css and inject.bundle.css 😐.

Once the PR is merged, this will require waiting for a new Sourcegraph release.

Other issues

The hover menu looks a bit gross, I'll verify how it is supposed to look in our browser extension before merging this PR.

Test plan

Screenshot 2022-11-03 at 15 05 28

App preview:

Check out the client app preview documentation to learn more.

@philipp-spiess philipp-spiess requested review from a team and valerybugakov November 3, 2022 14:09
@philipp-spiess philipp-spiess self-assigned this Nov 3, 2022
@cla-bot cla-bot Bot added the cla-signed label Nov 3, 2022

@valerybugakov valerybugakov left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Good catch!🔥

@philipp-spiess

Copy link
Copy Markdown
Contributor Author

Added some docs as well to the browser extensions README so that we have an easier time reproducing this the next time 🙂

@sg-e2e-regression-test-bob

sg-e2e-regression-test-bob commented Nov 3, 2022

Copy link
Copy Markdown

Bundle size report 📦

Initial size Total size Async size Modules
0.00% (0.00 kb) 0.00% (0.00 kb) 0.00% (0.00 kb) 0.00% (0)

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

@vdavid vdavid left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Wow, great docs!

@philipp-spiess

Copy link
Copy Markdown
Contributor Author

Apparently this is the expected styling, this is from the browser extension:

Screenshot 2022-11-03 at 15 34 02

@philipp-spiess philipp-spiess merged commit 52fc286 into main Nov 3, 2022
@philipp-spiess philipp-spiess deleted the ps/fix-phabricator-native-extension branch November 3, 2022 14:59
philipp-spiess added a commit that referenced this pull request Nov 8, 2022
philipp-spiess added a commit that referenced this pull request Nov 8, 2022
@philipp-spiess philipp-spiess mentioned this pull request Nov 8, 2022
11 tasks
philipp-spiess added a commit that referenced this pull request Nov 8, 2022
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.

Sourcegraph phabricartor extension blocked by CORS policy

4 participants