weblue / wanna_b_tierlist

http://cephalonwannab.com
Apache License 2.0
34 stars 9 forks source link

Upgrading Bootstrap/Font Awesome #33

Closed mattxwang closed 6 years ago

mattxwang commented 6 years ago

I'm a big fan of using the website - the design itself is pretty efficient, which is something that I quite appreciate. However, I noticed that you're using Bootstrap 3 (and Font Awesome 4) - have you considered updating them to Bootstrap 4 and Font Awesome 5 respectively? It's not an immediate change, but I think it'll be nice to give it a facelift (and reap a few small performance benefits). Off of the top of my head, I think you'll have a sleeker design (subjective), better print classes/responsiveness, flex utilities, better form controls/styling, and more customization over minor style stuff.

As an aside, you could also custom-compile your own version of Bootstrap that doesn't have all the features that you don't use, which can lower page load. You can also use slim jQuery, which at a quick glance will still fit all your needs without being as complicated.

I don't mind helping out (by submitting a few PRs) - what do you think?

weblue commented 6 years ago

I’m completely open to a PR to change the design dependencies, I actually had very little involvement in the html/css of the website so any and all changes are welcome pending a code review.

Please feel free to change what you’d like and let me know if there’s any issues with integrating the angular functionality

Also, I’m on vacation right now so the PR may take a while to get approved, but I’ll review it as soon as I’m able to

On Aug 29, 2018, at 1:12 PM, Matthew Wang notifications@github.com wrote:

I'm a big fan of using the website - the design itself is pretty efficient, which is something that I quite appreciate. However, I noticed that you're using Bootstrap 3 (and Font Awesome 4) - have you considered updating them to Bootstrap 4 and Font Awesome 5 respectively? It's not an immediate change, but I think it'll be nice to give it a facelift (and reap a few small performance benefits). Off of the top of my head, I think you'll have a sleeker design (subjective), better print classes/responsiveness, flex utilities, better form controls/styling, and more customization over minor style stuff.

As an aside, you could also custom-compile your own version of Bootstrap that doesn't have all the features that you don't use, which can lower page load. You can also use slim jQuery, which at a quick glance will still fit all your needs without being as complicated.

I don't mind helping out (by submitting a few PRs) - what do you think?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

mattxwang commented 6 years ago

Sounds good, I'll put in some work when I can and send some PRs your way. I hope you enjoy your vacation!

mattxwang commented 6 years ago

Hey @weblue, hope the vacation is good so far. Just wanted to check in with some progress: after an initial passthrough, the site looks like this: https://malsf21.github.io/weblue.github.io/

I'm still fixing a few things (issues with mobile responsiveness, minor design annoyances when transferring over from BS3 -> BS4), but I just wanted to ask and see if there are any major issues you see with the design before I send in a PR.

On a sidenote (as in it wouldn't be covered in this PR), but have you thought about making the jQuery AJAX call asynchronous? jQuery has already deprecated the synchronous AJAX call (and it's gone in the latest slim release), so if the site can work in async then it should be changed eventually.

Let me know what you think!

weblue commented 6 years ago

The design looks great! I'm gonna start looking into removing the ajax call entirely and using angular, as that's the current reason that it's synchronous.

On Fri, Sep 7, 2018 at 1:54 AM Matthew Wang notifications@github.com wrote:

Hey @weblue https://github.com/weblue, hope the vacation is good so far. Just wanted to check in with some progress: after an initial passthrough, the site looks like this: https://malsf21.github.io/weblue.github.io/

I'm still fixing a few things (issues with mobile responsiveness, minor design annoyances when transferring over from BS3 -> BS4), but I just wanted to ask and see if there are any major issues you see with the design before I send in a PR.

On a sidenote (as in it wouldn't be covered in this PR), but have you thought about making the jQuery AJAX call asynchronous? jQuery has already deprecated the synchronous AJAX call (and it's gone in the latest slim release), so if the site can work in async then it should be changed eventually.

Let me know what you think!

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/weblue/weblue.github.io/issues/33#issuecomment-419331110, or mute the thread https://github.com/notifications/unsubscribe-auth/AAgzq7dfHBFIdacFe-WJZkPz3fcywfJ8ks5uYgomgaJpZM4WQy1h .

--

Nader Kirrish Phone: (571) 242-4633

mattxwang commented 6 years ago

Sounds good, I'll finish up those final few fixes and send a pull request your way.

If you want a native solution to replace the AJAX call, you could look into the Javascript Fetch API which seems like a direct upgrade to the good old XMLHttpRequest. I'm sure that there are also Angular wrappers that do the same thing.

mattxwang commented 6 years ago

Hey - I've finished up all of my changes on the index page (other than one pesky thing about a default active pane, though that might just have to stick). However, I was doing some work on the editor page, but then I realised that the hyper-compactness might be a feature worth keeping. Do you mind taking a look? Is the more spaced-out Alerts section better than the following sections (which I admit are kind of a layout nightmare).

Anyways, let me know what you think - at this point, any PR would be good for production!

weblue commented 6 years ago

The editor page was untouched as Sakai4eva decided to just do his edits by changing the JSON file so I never looked at it again. It definitely deserves a touch up. The alerts section definitely looks better and a way to collapse or choose a section would also improve it. Feel free to make any changes you think will help

mattxwang commented 6 years ago

I've re-formatted the editor page, using your suggestion of collapsing sections. I think it looks a lot cleaner! You can see the updated version here.

Is there anything else you think I should take a look at before I submit the PR?

weblue commented 6 years ago

Everything looks good! Be sure to add yourself to the "Thanks to" section as well as linking any social media or website you'd like to attach to yourself

mattxwang commented 6 years ago

Awesome, have done! I've sent in the PR at #35!

weblue commented 6 years ago

Just merged the PR, will let the changes sit for a couple of days before closing the issue to allow for user testing and see if there's any bugs.

I appreciate all the work you put into this, lmk if you're on the Discord and we'll get you an exclusive role on there!

mattxwang commented 6 years ago

Sounds good! I'm in the discord, I'm Matthew Wang#9698. Thanks for the help!

weblue commented 6 years ago

Messaged you on discord, but just for the sake of issue tracking:

There's an issue in an element that's causing the website to show a horizontal scroll bar

weblue commented 6 years ago

Also, looks like the hidden class used for hiding some alerts is broken. This bug isn't a huge deal since those can just be removed from the JSON db

weblue commented 6 years ago

The loading screen seems to not be covering a small part of the top section of the website