search

andrewsomething / bug2trello

A Chrome extension to add bugs/issues to a Trello board
Other
22 stars 9 forks source link

Extension overhaul #19

Closed eoger closed 7 years ago

eoger commented 7 years ago

This brings:

Hope you like it!

andrewsomething commented 7 years ago

@eoger This is great stuff. I need to give it a more thorough review before merging, but this definitely looks like a big improvement. I like the approach of breaking out "providers" into separate files.

andrewsomething commented 7 years ago

So a few questions before merging:

I'd love to hear why you think removing Bootstrap is an improvement. I know it's subjective, but I personally think the look and feel is a bit better with it.

I'm curious about the project layout and build process. Some of the files seem to be auto-generated from webpack. I'd rather not have them in version control. I think it might be better to use something like gulp to build everything into a dist directory.

eoger commented 7 years ago

I'd love to hear why you think removing Bootstrap is an improvement. I know it's subjective, but I personally think the look and feel is a bit better with it.

The shapes and colors are definitely subjective, and I'd be happy if someone with a better taste than me proposes a better design.
Regarding Bootstrap, in my opinion including 1mb of CSS for less than ten div is overkill, if we really like the Bootstrap default design we can still reproduce it ;)

I'm curious about the project layout and build process. Some of the files seem to be auto-generated from webpack. I'd rather not have them in version control. I think it might be better to use something like gulp to build everything into a dist directory.

Could you point these auto-generated files to me? I made sure to include them in the .gitignore but I might have missed something.

The "dist" folder you're mentioning is basically the addon folder, everything parent to that folder should (and will) not be included in the resulting zip addon file.

The build process starts with 2 root files: options/options.js and popup/popup.js.
These files are bundled with their dependencies and end up in the addon/popup and addon/options as index.js files (referenced in the .html files).

Thanks a lot for taking the time to review this!

andrewsomething commented 7 years ago

Could you point these auto-generated files to me? I made sure to include them in the .gitignore but I might have missed something.

I missed the .gitignore in the addons folder. Though I do still think it's cleaner to build into a separate directory.

Regarding Bootstrap, in my opinion including 1mb of CSS for less than ten div is overkill,

OK, you've sold me. :smile:

I'm going to go ahead and merge this now. I've also added you as a contributor to the repo. Though I think I'll probably want to do some theming changes before pushing it out to the Chrome Web Store.

Thanks for all of your work on this!

eoger commented 7 years ago

Awesome thanks!
I'll open an issue once Firefox runs bug2trello without hiccups.