Bug: change in size GitHub (UX)

[a0m0rajab]

Describe the bug

When you open GitHub with the extension enabled it resizes the screen and change dimension which is a bit uncomfortable.

Steps to reproduce

1. enable the extension.
2. go to: Feature: add PR to highlights from the extension #88
3. you see a minor resize at the beginning, (you might need to force refresh to see the issue (CTRL+SHIFT+R))
4. disable the extension
5. go to link in step 2
6. you see there is no resize

www_screencapture_com_2023-5-18_18_12.mp4

Affected services

browser-extension

Platforms

No response

Browsers

No response

Environment

No response

Additional context

No response

Code of Conduct

• I agree to follow this project's Code of Conduct

Contributing Docs

• I agree to follow this project's Contribution Docs

[Nanak360]

Yes, I have noticed this issue as well.
this is happening because of importing @tailwind utilities; in content-scripts.css

I also noticed issues on the issue page...hehe, get it?

notice the issue button on the above image. It's not aligned properly.
This happens when we have @tailwind utilities; imported

BUT
if we remove @tailwind utilities; from the CSS sheet, the whole issue disappears but all the OpenSauced components look bad. Really bad.

Maybe if we configure tailwind.config.js to match with github's tailwind config or something, we can solve this issue.
Another solution can be defining our own CSS classes and using them.

[a0m0rajab]

Thank you for your comment! I think this might be a thing that @Anush008 or @diivi can discuss better than me.
From my perspective, I think writing our own CSS class would be better and cause no conflict in the future. Even if anything changes on GitHub, it will be dependency free.

[Nanak360]

writing our own CSS class would be better and cause no conflict in the future

I agree with you on this. Currently, there are 11 components being injected into GitHub, and it would indeed be relatively easy to modify them to use custom CSS classes. However, as the number of components grows, converting them all to custom CSS classes could become more challenging and time-consuming.

[a0m0rajab]

This Stackoverflow question might help, I quickly checked it but did not fully test the situation.

[Nanak360]

This Stackoverflow question might help, I quickly checked it but did not fully test the situation.

This looks really promising

[a0m0rajab]

The solution I shared previously resolves the conflict, but since it's adding a prefix to tailwind, the react app gots broken. I researched further and found that we can configure react to add a prefix to classNames.

Some libraries are adding the prefix to className like this, and this one. Some people used to do it manually but I do prefer a library not to get out of the standard practices. Here is a related question in StackOverFLow and unanswered question in stackoverflow,

Another thing we can do is that since the project is using PostCSS we might add the prefix there.

A library to try, PostCSS library, postCSS prefixer, and PostCSS related StackOverFlow Question mentioning the library and showing an example of it.

I think the next step here would be trying both solutions,

1. add prefix to tailwind
2. add prefix to react classNames
3. check the pure HTML (content scripts) result - this might be the tricky one -

[marcusgchan]

Another possible solution could be creating a shadow dom for each element that is inserted into the page since it will isolate styling from the host page.

[marcusgchan]

.take

[Anush008]

Hey @marcusgchan, about the use of a shadow DOM.The injected elements inherit some of their styling from GitHub itself. The shadow DOM will obstruct this behaviour.

[marcusgchan]

I see, didn't realize that it was using some of the styling from GitHub

[marcusgchan]

I looked at the react-classname-prefix-loader and it seems interesting since it automates the process of adding prefixes during the build phase, but there are a few things that concern me:

1. It hasn't been updated in 6 years
2. There's a bug that incorrectly adds prefixes to attributes
3. It adds a hidden abstraction where all classes will be prefixed unless added to the blacklist
4. I'm not sure if there's a simple way to detect if a class should have a prefix for content scripts since it uses a mixture of github and tailwind classes

It may be easier to manually add the prefix, but then it's a bit tedious to add "tw-" to every class in the future. Any suggestions?

[bdougie]

My suggestion see if there is a modern alternative. Perhaps create a vite plugin.

The alternative is creating our own prefixer in the lib folder to make our css collision proof.

[Anush008]

I tried out building the plugin. Prefixing works as we'd expect it to.
https://gist.github.com/Anush008/ffebc6ceb5439dc1ca2996ff7fb88311

The problem being, some of our components inherit styling from GitHub using their classnames. Like SelectMenu js-slash-command-menu in the following example.

ai/src/content-scripts/components/AICodeReview/AICodeReviewMenu.ts

Line 22 in e95e7a1

className: "SelectMenu js-slash-command-menu hidden mt-6",

ai/src/content-scripts/components/InsightsSelectDropdown/InsightsSelectDropdown.ts

Line 12 in e95e7a1

<div class="SelectMenu-modal">

ai/src/content-scripts/components/InsightsSelectDropdown/InsightsSelectDropdown.ts

Line 14 in e95e7a1

<h5 class="SelectMenu-title f5 text-left">Add Repo To Insights</h5>

Prefixing will break this behaviour.