Skip to content
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.

Sign up for GitHub

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

Jump to bottom

fix: re-enable tailwind base class on extension popup #31

Merged
merged 3 commits into from
Apr 25, 2023
Merged

fix: re-enable tailwind base class on extension popup #31

merged 3 commits into from
Apr 25, 2023

Conversation

doaortu
Copy link
Contributor

@doaortu doaortu commented Apr 24, 2023

What type of PR is this? (check all applicable)

  • 🍕 Feature
  • 🐛 Bug Fix
  • 📝 Documentation Update
  • 🎨 Style
  • 🧑‍💻 Code Refactor
  • 🔥 Performance Improvements
  • ✅ Test
  • 🤖 Build
  • 🔁 CI
  • 📦 Chore (Release)
  • ⏩ Revert

Description

This PR is made based on this discussion.
Summary, tailwind base classes is omitted because it's interfered with github native styling in content-scripts. The issue is solved, but I have another concern regarding how we write tailwind classes, it's felt unintuitive when writing tailwind without base classes (e.g. when we want to specify tailwind divider, we usually go with divide-y divide-white, but when we disable the base directives, we need to add new classes like so divide-y divide-white divide-solid divide-x-0 ).
Another concern is to make it easier for new contributor to get up quickly when working with this repository, because most projects, most tutorial, most stackoveflow answers are assumed we use tailwind base classes.

Hence I re-enabling the tailwind base classes for the extension popup, when we have no concern about styling conflict.
On the discussion before, I stated that we have 2 options, I'll go with spliting the base css into two (one for content-script and one for extension popup, because that seems easy and quick workaround. I can go try to implement the Shadow DOM approach later if we think that's a better idea.)

Related Tickets & Documents

This PR is related to this discussion

Mobile & Desktop Screenshots/Recordings

None

Added tests?

  • 👍 yes
  • 🙅 no, because they aren't needed
  • 🙋 no, because I need help

Added to documentation?

  • 📜 README.md
  • 📓 docs.opensauced.pizza
  • 🍕 dev.to/opensauced
  • 📕 storybook
  • 🙅 no documentation needed

[optional] Are there any post-deployment tasks we need to perform?

[optional] What gif best describes this PR or how it makes you feel?

@kuwakuwi1
fix: re-enable tailwind base class on
4e72936 
extension popup

because writing tailwind without base classes feels unintuitive
@doaortu
Copy link
Contributor Author

doaortu commented Apr 24, 2023

I have another concern when writing this PR,
I don't know if it's okay to just copy paste the index.css to content-scripts.css just like that (something about DRY principle?)

and, is it better for us to move the content-script related components to the content-scripts folder? Because we now have 2 different base css.

What do you guys think @diivi @Anush008 ?

@Anush008
Copy link
Member

Anush008 commented Apr 24, 2023 •
edited
Loading

@doaortu, moving it to the content-scripts dir seems better. Also, I wouldn't bother about DRY here so much since the repetition is trivial.
Edit: By "it", I meant the new CSS file.

@doaortu
Copy link
Contributor Author

doaortu commented Apr 24, 2023

I agree with you.
How about you @diivi ?

@diivi
Copy link
Contributor

diivi commented Apr 24, 2023

Yup, better according to me too.
One more thing, maybe you can add comments to the top of these files to let other devs know what the difference between them is, or why they exist at the first place.

@diivi
Copy link
Contributor

diivi commented Apr 24, 2023

Or index.css can be renamed to popup.css

@doaortu
Copy link
Contributor Author

doaortu commented Apr 25, 2023

How about now? @diivi @Anush008 ?

@Anush008
Copy link
Member

Anush008 commented Apr 25, 2023 •
edited
Loading

@doaortu, the components dir will house components to be used outside of content scripts too. I recommend only moving content-scripts.css to the content-scripts dir.

@doaortu
Copy link
Contributor Author

doaortu commented Apr 25, 2023 •
edited
Loading

I think you misundestood my intention here.
I don't meant to move the components dir to content-scripts , but I'm adding the components dir to content-scripts folder, so we end up with 2 components dir, one in src and other one in content-scripts folder, indicating that components in content-scripts folder should be used in content-scripts context only, to avoid misusage.

@Anush008
Copy link
Member

Anush008 commented Apr 25, 2023 •
edited
Loading

@doaortu, thank you for clarifying the changes. The moving of the current components directory from src makes sense now, as the components in that folder were intended to be used with content-scripts which require HTML elements made using document.createElement().
However, the extension drop-down will be built using React, which can only use React elements and not HTML elements.
Therefore, it seems appropriate to have a separate components directory for React components in src, and a content-scripts/components directory for HTML elements to be used in content-scripts.
This approach will help ensure that the right types of components are present in the right places.

@doaortu
Copy link
Contributor Author

doaortu commented Apr 25, 2023

Yup, thank you for adding more details on my previous description.
Thank you for approving the changes.

@Anush008 Anush008 requested review from diivi and bdougie April 25, 2023 16:28
@bdougie bdougie merged commit c081217 into open-sauced:main Apr 25, 2023
github-actions bot pushed a commit that referenced this pull request Apr 27, 2023
@diivi @open-sauced
chore(minor): release 1.0.0-beta.1 on beta channel [skip ci]
e872f55 
## 1.0.0-beta.1 (2023-04-27)

### 📝 Documentation

* README and LICENSE ([917f7b0](917f7b0))
* Updated the README.md ([#15](#15)) ([3e4dd69](3e4dd69))

### 🐛 Bug Fixes

*  rename the. extension ([294e499](294e499))
*  Update URLs ([5828e20](5828e20))
* make emails less creepy ([a08fa45](a08fa45))
* re-enable tailwind base class on extension popup ([#31](#31)) ([c081217](c081217))
* Wrapped the await call in an async() ([#26](#26)) ([357aab5](357aab5))

### 🍕 Features

* add home page and profile page ([#36](#36)) ([8a43288](8a43288))
* Added a configuration file ([#32](#32)) ([ec6722c](ec6722c))
* Auto PAT authentication ([#29](#29)) ([f4f8cf6](f4f8cf6))
* extension window navigation ([#21](#21)) ([b5ae9a6](b5ae9a6))
* GitHub page update listener ([#37](#37)) ([06b070f](06b070f))
* Invite to OpenSauced ([#20](#20)) ([0c8f222](0c8f222))
* PAT authentication ([#27](#27)) ([e726a93](e726a93))
* updated the extension logo ([#22](#22)) ([d19034d](d19034d))
* view on OpenSauced button ([#14](#14)) ([a447551](a447551))

### 🔁 Continuous Integration

* automated github releases ([#39](#39)) ([735fa66](735fa66)), closes [#16](#16)
@github-actions
Copy link

🎉 This PR is included in version 1.0.0-beta.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

github-actions bot pushed a commit to diivi/browser-extensions that referenced this pull request Apr 27, 2023
@diivi @open-sauced
chore(minor): release 1.0.0 [skip ci]
4fe62d9 
## 1.0.0 (2023-04-27)

### 📝 Documentation

* README and LICENSE ([917f7b0](917f7b0))
* Updated the README.md ([open-sauced#15](https://github.com/diivi/browser-extensions/issues/15)) ([3e4dd69](3e4dd69))

### 🐛 Bug Fixes

*  rename the. extension ([294e499](294e499))
*  Update URLs ([5828e20](5828e20))
* make emails less creepy ([a08fa45](a08fa45))
* re-enable tailwind base class on extension popup ([open-sauced#31](https://github.com/diivi/browser-extensions/issues/31)) ([c081217](c081217))
* Wrapped the await call in an async() ([open-sauced#26](https://github.com/diivi/browser-extensions/issues/26)) ([357aab5](357aab5))

### 🍕 Features

* add home page and profile page ([open-sauced#36](https://github.com/diivi/browser-extensions/issues/36)) ([8a43288](8a43288))
* Added a configuration file ([open-sauced#32](https://github.com/diivi/browser-extensions/issues/32)) ([ec6722c](ec6722c))
* Auto PAT authentication ([open-sauced#29](https://github.com/diivi/browser-extensions/issues/29)) ([f4f8cf6](f4f8cf6))
* extension window navigation ([open-sauced#21](https://github.com/diivi/browser-extensions/issues/21)) ([b5ae9a6](b5ae9a6))
* GitHub page update listener ([open-sauced#37](https://github.com/diivi/browser-extensions/issues/37)) ([06b070f](06b070f))
* Invite to OpenSauced ([open-sauced#20](https://github.com/diivi/browser-extensions/issues/20)) ([0c8f222](0c8f222))
* PAT authentication ([open-sauced#27](https://github.com/diivi/browser-extensions/issues/27)) ([e726a93](e726a93))
* updated the extension logo ([open-sauced#22](https://github.com/diivi/browser-extensions/issues/22)) ([d19034d](d19034d))
* view on OpenSauced button ([open-sauced#14](https://github.com/diivi/browser-extensions/issues/14)) ([a447551](a447551))
github-actions bot pushed a commit to diivi/browser-extensions that referenced this pull request Apr 27, 2023
@diivi @open-sauced
chore(minor): release 1.0.0 [skip ci]
2250608 
## 1.0.0 (2023-04-27)

### 📝 Documentation

* README and LICENSE ([917f7b0](917f7b0))
* Updated the README.md ([open-sauced#15](https://github.com/diivi/browser-extensions/issues/15)) ([3e4dd69](3e4dd69))

### 🐛 Bug Fixes

*  rename the. extension ([294e499](294e499))
*  Update URLs ([5828e20](5828e20))
* make emails less creepy ([a08fa45](a08fa45))
* re-enable tailwind base class on extension popup ([open-sauced#31](https://github.com/diivi/browser-extensions/issues/31)) ([c081217](c081217))
* Wrapped the await call in an async() ([open-sauced#26](https://github.com/diivi/browser-extensions/issues/26)) ([357aab5](357aab5))

### 🍕 Features

* add home page and profile page ([open-sauced#36](https://github.com/diivi/browser-extensions/issues/36)) ([8a43288](8a43288))
* Added a configuration file ([open-sauced#32](https://github.com/diivi/browser-extensions/issues/32)) ([ec6722c](ec6722c))
* Auto PAT authentication ([open-sauced#29](https://github.com/diivi/browser-extensions/issues/29)) ([f4f8cf6](f4f8cf6))
* extension window navigation ([open-sauced#21](https://github.com/diivi/browser-extensions/issues/21)) ([b5ae9a6](b5ae9a6))
* GitHub page update listener ([open-sauced#37](https://github.com/diivi/browser-extensions/issues/37)) ([06b070f](06b070f))
* Invite to OpenSauced ([open-sauced#20](https://github.com/diivi/browser-extensions/issues/20)) ([0c8f222](0c8f222))
* PAT authentication ([open-sauced#27](https://github.com/diivi/browser-extensions/issues/27)) ([e726a93](e726a93))
* updated the extension logo ([open-sauced#22](https://github.com/diivi/browser-extensions/issues/22)) ([d19034d](d19034d))
* view on OpenSauced button ([open-sauced#14](https://github.com/diivi/browser-extensions/issues/14)) ([a447551](a447551))
github-actions bot pushed a commit to diivi/browser-extensions that referenced this pull request Apr 27, 2023
@diivi @open-sauced
chore(minor): release 1.0.0 [skip ci]
055c832 
## 1.0.0 (2023-04-27)

### 📝 Documentation

* README and LICENSE ([917f7b0](917f7b0))
* Updated the README.md ([open-sauced#15](https://github.com/diivi/browser-extensions/issues/15)) ([3e4dd69](3e4dd69))

### 🐛 Bug Fixes

*  rename the. extension ([294e499](294e499))
*  Update URLs ([5828e20](5828e20))
* make emails less creepy ([a08fa45](a08fa45))
* re-enable tailwind base class on extension popup ([open-sauced#31](https://github.com/diivi/browser-extensions/issues/31)) ([c081217](c081217))
* Wrapped the await call in an async() ([open-sauced#26](https://github.com/diivi/browser-extensions/issues/26)) ([357aab5](357aab5))

### 🍕 Features

* add home page and profile page ([open-sauced#36](https://github.com/diivi/browser-extensions/issues/36)) ([8a43288](8a43288))
* Added a configuration file ([open-sauced#32](https://github.com/diivi/browser-extensions/issues/32)) ([ec6722c](ec6722c))
* Auto PAT authentication ([open-sauced#29](https://github.com/diivi/browser-extensions/issues/29)) ([f4f8cf6](f4f8cf6))
* extension window navigation ([open-sauced#21](https://github.com/diivi/browser-extensions/issues/21)) ([b5ae9a6](b5ae9a6))
* GitHub page update listener ([open-sauced#37](https://github.com/diivi/browser-extensions/issues/37)) ([06b070f](06b070f))
* Invite to OpenSauced ([open-sauced#20](https://github.com/diivi/browser-extensions/issues/20)) ([0c8f222](0c8f222))
* PAT authentication ([open-sauced#27](https://github.com/diivi/browser-extensions/issues/27)) ([e726a93](e726a93))
* updated the extension logo ([open-sauced#22](https://github.com/diivi/browser-extensions/issues/22)) ([d19034d](d19034d))
* view on OpenSauced button ([open-sauced#14](https://github.com/diivi/browser-extensions/issues/14)) ([a447551](a447551))
github-actions bot pushed a commit that referenced this pull request May 13, 2023
@bdougie @open-sauced
chore(minor): release 1.0.0 [skip ci]
d49015d 
## 1.0.0 (2023-05-13)

### 📝 Documentation

* README and LICENSE ([917f7b0](917f7b0))
* Updated the README.md ([#15](#15)) ([3e4dd69](3e4dd69))

### 🔁 Continuous Integration

* automated github releases ([#39](#39)) ([735fa66](735fa66)), closes [#16](#16)

### 🧑‍💻 Code Refactoring

* improved-auth-strategy ([#56](#56)) ([d8b27c3](d8b27c3))

### 🍕 Features

* add home page and profile page ([#36](#36)) ([8a43288](8a43288))
* add to highlights button ([#64](#64)) ([e2120a1](e2120a1))
* Added a configuration file ([#32](#32)) ([ec6722c](ec6722c))
* Added light mode to GH profile buttons ([#40](#40)) ([a7ea43a](a7ea43a))
* Added linting to the project. ([#49](#49)) ([1f73007](1f73007))
* Auto PAT authentication ([#29](#29)) ([f4f8cf6](f4f8cf6))
* extension window navigation ([#21](#21)) ([b5ae9a6](b5ae9a6))
* GitHub page update listener ([#37](#37)) ([06b070f](06b070f))
* Invite to OpenSauced ([#20](#20)) ([0c8f222](0c8f222))
* PAT authentication ([#27](#27)) ([e726a93](e726a93))
* updated the extension logo ([#22](#22)) ([d19034d](d19034d))
* view on OpenSauced button ([#14](#14)) ([a447551](a447551))
* vote repos ([#70](#70)) ([ff2c8f4](ff2c8f4))

### 🐛 Bug Fixes

*  rename the. extension ([294e499](294e499))
*  Update URLs ([5828e20](5828e20))
* attach dist folder to release assets ([#47](#47)) ([4f50e72](4f50e72))
* circular-deps in vote buttons ([675496c](675496c))
* Duplicate buttons on GH profile page ([#48](#48)) ([9ee2dca](9ee2dca))
* incorrect request for voting ([#72](#72)) ([7a35014](7a35014))
* make emails less creepy ([a08fa45](a08fa45))
* re-enable tailwind base class on extension popup ([#31](#31)) ([c081217](c081217))
* remove activeTab manifest.json ([#50](#50)) ([dd71480](dd71480))
* render emojis in user bio ([#68](#68)) ([d0158a5](d0158a5))
* set-output deprecation warning ([#51](#51)) ([486f84b](486f84b))
* unrecognized-manifest-key ([#55](#55)) ([d622500](d622500))
* Wrapped the await call in an async() ([#26](#26)) ([357aab5](357aab5))
@github-actions
Copy link

🎉 This PR is included in version 1.0.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

zer0and1 pushed a commit to zer0and1/open-sauced.ai that referenced this pull request Jul 26, 2023
@diivi @open-sauced
chore(minor): release 1.0.0-beta.1 on beta channel [skip ci]
64275bc 
## 1.0.0-beta.1 (2023-04-27)

### 📝 Documentation

* README and LICENSE ([917f7b0](open-sauced/ai@917f7b0))
* Updated the README.md ([#15](open-sauced/ai#15)) ([3e4dd69](open-sauced/ai@3e4dd69))

### 🐛 Bug Fixes

*  rename the. extension ([294e499](open-sauced/ai@294e499))
*  Update URLs ([5828e20](open-sauced/ai@5828e20))
* make emails less creepy ([a08fa45](open-sauced/ai@a08fa45))
* re-enable tailwind base class on extension popup ([#31](open-sauced/ai#31)) ([c081217](open-sauced/ai@c081217))
* Wrapped the await call in an async() ([#26](open-sauced/ai#26)) ([357aab5](open-sauced/ai@357aab5))

### 🍕 Features

* add home page and profile page ([#36](open-sauced/ai#36)) ([8a43288](open-sauced/ai@8a43288))
* Added a configuration file ([#32](open-sauced/ai#32)) ([ec6722c](open-sauced/ai@ec6722c))
* Auto PAT authentication ([#29](open-sauced/ai#29)) ([f4f8cf6](open-sauced/ai@f4f8cf6))
* extension window navigation ([#21](open-sauced/ai#21)) ([b5ae9a6](open-sauced/ai@b5ae9a6))
* GitHub page update listener ([#37](open-sauced/ai#37)) ([06b070f](open-sauced/ai@06b070f))
* Invite to OpenSauced ([#20](open-sauced/ai#20)) ([0c8f222](open-sauced/ai@0c8f222))
* PAT authentication ([#27](open-sauced/ai#27)) ([e726a93](open-sauced/ai@e726a93))
* updated the extension logo ([#22](open-sauced/ai#22)) ([d19034d](open-sauced/ai@d19034d))
* view on OpenSauced button ([#14](open-sauced/ai#14)) ([a447551](open-sauced/ai@a447551))

### 🔁 Continuous Integration

* automated github releases ([#39](open-sauced/ai#39)) ([735fa66](open-sauced/ai@735fa66)), closes [#16](open-sauced/ai#16)
zer0and1 pushed a commit to zer0and1/open-sauced.ai that referenced this pull request Jul 26, 2023
@bdougie @open-sauced
chore(minor): release 1.0.0 [skip ci]
f1ca2dd 
## 1.0.0 (2023-05-13)

### 📝 Documentation

* README and LICENSE ([917f7b0](open-sauced/ai@917f7b0))
* Updated the README.md ([#15](open-sauced/ai#15)) ([3e4dd69](open-sauced/ai@3e4dd69))

### 🔁 Continuous Integration

* automated github releases ([#39](open-sauced/ai#39)) ([735fa66](open-sauced/ai@735fa66)), closes [#16](open-sauced/ai#16)

### 🧑‍💻 Code Refactoring

* improved-auth-strategy ([#56](open-sauced/ai#56)) ([d8b27c3](open-sauced/ai@d8b27c3))

### 🍕 Features

* add home page and profile page ([#36](open-sauced/ai#36)) ([8a43288](open-sauced/ai@8a43288))
* add to highlights button ([#64](open-sauced/ai#64)) ([e2120a1](open-sauced/ai@e2120a1))
* Added a configuration file ([#32](open-sauced/ai#32)) ([ec6722c](open-sauced/ai@ec6722c))
* Added light mode to GH profile buttons ([#40](open-sauced/ai#40)) ([a7ea43a](open-sauced/ai@a7ea43a))
* Added linting to the project. ([#49](open-sauced/ai#49)) ([1f73007](open-sauced/ai@1f73007))
* Auto PAT authentication ([#29](open-sauced/ai#29)) ([f4f8cf6](open-sauced/ai@f4f8cf6))
* extension window navigation ([#21](open-sauced/ai#21)) ([b5ae9a6](open-sauced/ai@b5ae9a6))
* GitHub page update listener ([#37](open-sauced/ai#37)) ([06b070f](open-sauced/ai@06b070f))
* Invite to OpenSauced ([#20](open-sauced/ai#20)) ([0c8f222](open-sauced/ai@0c8f222))
* PAT authentication ([#27](open-sauced/ai#27)) ([e726a93](open-sauced/ai@e726a93))
* updated the extension logo ([#22](open-sauced/ai#22)) ([d19034d](open-sauced/ai@d19034d))
* view on OpenSauced button ([#14](open-sauced/ai#14)) ([a447551](open-sauced/ai@a447551))
* vote repos ([#70](open-sauced/ai#70)) ([ff2c8f4](open-sauced/ai@ff2c8f4))

### 🐛 Bug Fixes

*  rename the. extension ([294e499](open-sauced/ai@294e499))
*  Update URLs ([5828e20](open-sauced/ai@5828e20))
* attach dist folder to release assets ([#47](open-sauced/ai#47)) ([4f50e72](open-sauced/ai@4f50e72))
* circular-deps in vote buttons ([675496c](open-sauced/ai@675496c))
* Duplicate buttons on GH profile page ([#48](open-sauced/ai#48)) ([9ee2dca](open-sauced/ai@9ee2dca))
* incorrect request for voting ([#72](open-sauced/ai#72)) ([7a35014](open-sauced/ai@7a35014))
* make emails less creepy ([a08fa45](open-sauced/ai@a08fa45))
* re-enable tailwind base class on extension popup ([#31](open-sauced/ai#31)) ([c081217](open-sauced/ai@c081217))
* remove activeTab manifest.json ([#50](open-sauced/ai#50)) ([dd71480](open-sauced/ai@dd71480))
* render emojis in user bio ([#68](open-sauced/ai#68)) ([d0158a5](open-sauced/ai@d0158a5))
* set-output deprecation warning ([#51](open-sauced/ai#51)) ([486f84b](open-sauced/ai@486f84b))
* unrecognized-manifest-key ([#55](open-sauced/ai#55)) ([d622500](open-sauced/ai@d622500))
* Wrapped the await call in an async() ([#26](open-sauced/ai#26)) ([357aab5](open-sauced/ai@357aab5))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Anush008 Anush008 Anush008 approved these changes

@diivi diivi Awaiting requested review from diivi

@bdougie bdougie Awaiting requested review from bdougie

Assignees
No one assigned
Labels
released on @beta released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@doaortu @Anush008 @diivi @bdougie @kuwakuwi1