kblincoe / VisualGit_SE701_2019_1

1 stars 0 forks source link

Fixes #121 - Move bootstrap.css into stylesheets directory #174

Closed aorthi closed 5 years ago

aorthi commented 5 years ago

Related Issue/Keyword: Closes #121

Description: Pretty easy issue, just moved the bootstrap.css file into the stylesheets folder. Whoever does the review for this, please take the time to check that the styling hasn't been messed up anywhere. I spent quite a bit of time going through everything but there could have easily been something I missed.

![](insert gif link between braces here)

liamtbrand commented 5 years ago

While reviewing this I discovered that someone has merged something that seems to have broken the graphing functionality. I don't think it is to do with this but I wont merge yet until we figure out what broke that. See #87 .

liamtbrand commented 5 years ago

Ok, so moving this file doesn't appear to have broken anything when testing on my machine. Since no references to it were updated it seems that nothing would depend on it. Deleting the file also seemed to make no difference. Do we know why we needed this file in the first place? Maybe it would be better to remove it entirely. Has anyone any idea why it might be needed? - If not maybe deleting it is the best option.

emipeanz commented 5 years ago

@liamtbrand @aorthi I removed the file too and the app still ran fine, so maybe with a third to check we could delete the file entirely

aorthi commented 5 years ago

@liamtbrand @emipeanz I also tried removing bootstrap.css and found no changes, I also did a project-wide search for bootstrap.css and could not find any files that import anything from it. However, due to the nature of a central style sheet there isn't actually a way (that I can tell) to trace back where the styles are coming from aside from using dev tools on every single element within the project to check. I had a look around and weirdly enough, despite deleting the bootstrap.css file it still seems to apply its styles? Screen Shot 2019-04-01 at 12 48 03 PM

emipeanz commented 5 years ago

^^ Maybe its because its compiled with the style sheet, we delete it, but then dont compile it again??? Im not 100% sure how it all fits together

emipeanz commented 5 years ago

We could merge this PR and at a later date do a more thorough investigation into getting rid of the file altogether. Thoughts?

aorthi commented 5 years ago

We could merge this PR and at a later date do a more thorough investigation into getting rid of the file altogether. Thoughts?

@emipeanz Yeah I'm currently thinking that that might be the safest approach - merge this PR and open another issue to investigate what to do about the bootstrap.css file? I don't feel confident enough about bootstrap + npm compilation to make a call about removing it quite yet