jesperhodge / fabricator

A project management tool to coordinate open source teams.
3 stars 1 forks source link

Add frontend views #33

Closed angelocordon closed 6 years ago

angelocordon commented 6 years ago

Add dashboard front end views using Boostrap + React Handle: #30 #31

stain88 commented 6 years ago

Curious, any reason why you're not using a bootstrap gem instead of hard-coding the bootstrap dependencies?

angelocordon commented 6 years ago

Great comment @stain88 - I'll most likely move on to the gem eventually; for now since I'm not positive on our front end strategies yet, I thought this would be easiest to implement and take out. Plus, gives us the benefits of cached assets through CDN.

angelocordon commented 6 years ago

// 🚲shed:

@Aliin I don't disagree, DOM manipulation is typically considered an anti pattern these days and can be, in some cases, expensive memory-wise. However, none of the links you provided explains why it's "considered extremely bad practice" (unless I totally missed it?). The most helpful answers I've found are stuck all the way down at the bottom of the Quora link:

Bootstrap uses its own javascript which may conflict with react rendering of virtual dom... When react tries to render the dom its invokes external javascripts like material js or bootstrap js which causes extra overhead and can slow down the web app if it is content heavy. Using react components gives the developers the assurance that there wont be external calls to javascripts which might be incompatible with react dom renderer. Quora Answer

I am not sure you can actually use pure Bootstrap with React because I believe the Javascript in Bootstrap may manipulate the DOM and that would cause problems for React and how it detect changes and updates the DOM. Quora Answer

All in all, I think it just depends™.

But will check out ReactStrap though.

jesperhodge commented 6 years ago

I would still like us to switch to Reactstrap right away but I'm for merging this and doing all of that in a followup issue.

jesperhodge commented 6 years ago

Merging Checklist: