Improvements for the next release

[therealsujitk]

This issue is for discussions related to code clean ups & improvements.

[vatz88]

Looks good! How would it look on tab and mobile? Can you add a screenshot for that too?

Also, would it be better to put it on the right corner than left?

[therealsujitk]

There are two components of this application I'd like to improve,

• I've noticed the JavaScript needs a bit of cleaning and there are a lot of functions that are currently deprecated in the latest version of jQuery as well as JavaScript. For example keyCode and click().
• I'd also like to improve the design, this application currently uses Bootstrap 3.0, I'm working on upgrading it to Bootstrap 5.0, this would include a lot of design changes. Dark mode is quite popular lately so I thought why not try and implement it.

I'm currently working on the design first as it'll have a lot less conflicts during merging. I'll clean up the code and any unused resources once #173 has been properly resolved.

I've been thinking of improving the header to look like this.

I haven't gotten to the dark mode design yet, but it should look something like this.

[vatz88]

Design surely needs some improvements. Just to give to some historical context, the timetable you see, it has the exact colors as were present in the timetable in student login when I build this. This was to give users the same look and feel to which they are used to, when they see the timetable. I don't know have it looks in the student login now.

I'm very much in for improving the design but the thing with design is it's very subjective. I think you can start with adding a dark mode option. I'd say for the light mode, for now don't change any exiting color combinations. The nav bar you shared looks good! Just one more thing, make sure all changes are mobile friendly. We have good percentage of users using the site on mobile so I'd want to stick to mobile first principle.

[therealsujitk]

I don't know have it looks in the student login now.

It looks pretty much the same, VIT doesn't really change much, if something works leave it as is lol.

I'd say for the light mode, for now don't change any exiting color combinations.

I think it'll be best to keep the timetable colours the same. The design improvements will be mostly moving stuff around and adjusting border and margins (so there won't be any major colour changes, I might make the buttons look a bit darker in dark mode, again I'll have to see what looks best).

Just one more thing, make sure all changes are mobile friendly. We have good percentage of users using the site on mobile so I'd want to stick to mobile first principle.

I will also make sure it is responsive and super easy to use on mobile devices (ngl, I had no idea people used the mobile version that often, the desktop site is so much easier, but ig we have to ensure all devices can use it with ease). Thankfully Bootstrap takes care of this but I'll be sure to test it properly.

[vatz88]

I had no idea people used the mobile version that often

Yes, sure. We can merge these kind of improvements.

Yes! In fact stats are amazing to look at. Share you email id, I'll give you access to google analytics

[therealsujitk]

Yes! In fact stats are amazing to look at. Share you email id, I'll give you access to google analytics

Sure! My email: [Removed]

[therealsujitk]

I've grouped all the buttons like this. On mobile they'll show up one below the other (unless there's more space).

A modal will show up when the rename button is clicked. I've also added confirmation modals when a user resets or deletes a table.

If you have any suggestions let me know.

[vatz88]

Hey, the new design for buttons look good!

Two things:

1. Can we not show them one bellow another on small screens? Let's keep the scroll, otherwise it'll add lot of scroll in-between when you want to see the time table and then access the course search box.
2. Can we keep the edit and delete within the dropdown? That position is very self explanatory. It also lets you delete and edit table which you're not viewing. Also, let's keep the button attached to the dropdown for same reasons.

I like the modals, communicates better

[therealsujitk]

Alright, I'll see what I can do.

[therealsujitk]

Here's what it looks like.

Can we not show them one bellow another on small screens? Let's keep the scroll, otherwise it'll add lot of scroll in-between when you want to see the time table and then access the course search box.

I've implemented this like you asked, however I'm not a fan of scrolling buttons. I can't think of an improvement to this right now but hopefully I can come up with something. As of now it's scroll-able on mobile devices like you asked. I'm open to suggestions on improving this.

---

On a separate note, I won't be contributing for a while because of my FAT exams 😅, I'll continue redesigning and cleaning up everything next week after my exams are done.

[vatz88]

This looks good. Vertical scroll on buttons is consistent with current behaviour. Even I don't know what's best tbh.
We can merge this for now.

Thanks and all the best for your FAT!

[runofthemillgeek]

Great job @therealsujitk, did great work (been watching from the sidelines 👀 ) and you too, @vatz88 for juggling between day job and reviewing the work here even after all this time.

@vatz88 on a side note, should add a contributors section in the README, with those fancy circle avatars or the rectangle thingy. Maybe @therealsujitk or someone else can help add that thingy when they're available.

[vatz88]

should add a contributors section in the README

We can do that, not a super high prio though. GitHub already does a good work of highlighting contributors.

---

More importantly we need a CONTRIBUTING.md to help out new people willing to contribute

[runofthemillgeek]

Agreed! README prettifying and adding a CONTRIBUTING.md can be great good first issues! 🚀

[therealsujitk]

This is the design I came up with for the course slot selection section.

Previously there was a max limit on the number of slot buttons (i.e. 4) per row, I've gotten rid of that by adding display: flex; flex-wrap: wrap; on the container and using flex-grow: 1; width: 250px on the buttons, this way the buttons will fill up the page uniformly depending on the screen size.

Note: Ignore the lack of border around the Filter By Slot drop-down for now, there is some compatibility issue with Bootstrap 5 that I have to look into.

[vatz88]

Looks good! Are you migrating to bootstrap 5? If so, can you please raise the PR only with migration? I'd be easier to review. New design changes can be done in a follow up PR

[therealsujitk]

I wish it were that easy, the problem is in Bootstrap v5.0 a lot of changes have been made since v3.0 and that involves some components being removed as well. For example if you notice I've completely removed the blue panel in the slot selection section, that's because there is no longer a panel in Bootstrap 5.

When I first upgraded Bootstrap to v5.0 using yarn, several things broke, the colours for the timetable all became white, the drop-down's stopped working and that also includes the drop-down rendered by the bootstrap-multiselect package that is used to filter the slots. If I open a pull request only with the upgraded Bootstrap package, a lot of things will be broken and it wouldn't be possible to merge.

[vatz88]

Ah, makes sense. Do as you feel is easier. But try to break down into independent PRs when possible.
You can also keep a draft PR raised so I can have a look at the code and suggest any changes needed

[vatz88]

If it's making difficult to make things work with BS5 then we can stick with BS3. I know it's not easy to update, if it was easier to upgrade, i'd have done to BS4 by myself.

[therealsujitk]

But try to break down into independent PRs when possible.

Right now I'm only working on the design, so majority of the changes are in /src/index.html and /src/css/main.scss. Apart from this I've made a few JS changes to match with the design.

You can also keep a draft PR raised so I can have a look at the code and suggest any changes needed

Alright, I'll do that shortly. As of now most of design is complete, I'll just have to make some adjustments to make sure it's comfortable to use on all screens (larger as well as smaller). I haven't started the dark theme design yet, I'll open the PR for you to check if we can freeze the light theme UI first, then I'll proceed to design the dark theme.

If it's making difficult to make things work with BS5 then we can stick with BS3.

It's mostly done, I just have to see what's going on with the filter, there's probably an upgrade for the bootstrap-multiselect package for Bootstrap 5 compatibility. My exams just got over today, so I can get back to this now.

---

I'm planning to make a total of 3 PRs.

• The first PR (which I will open shortly) will only have the redesigned UI.
• The second PR will be removing deprecated, unused usages and redundant code. I also want to discuss on naming conventions, i.e what the names of the id's, classes and variable names should be (just to keep the code consistent). For example, element id's can be my-element (my personal preference) or myElement or My-Element, etc. Variables are currently camel cased, I think it should remain that way. After all this is done, I'm thinking it'll be a good idea to move the JS functions into separate files to make it easier to contribute in future (Ex: main.js, timetable.js, etc.). I'll open the PR before moving anything because if I do it after, git will treat them as new files and it'll be harder to review the changes.
• The third PR can be done by anyone, It'll involve updating README.md & CONTRIBUTING.md. Steps to install locally, where to look to make changes, stuff like that.

[vatz88]

Your 2nd point makes lot of sense!
(I wasn't smart enough back then to give give good variable names or follow good file naming conventions :P)

[therealsujitk]

For those scroll-able option buttons I was thinking of something like this.

Desktop:

Mobile:

[therealsujitk]

I've opened a draft PR (#179), deployed to Netlify.

[therealsujitk]

One of the major changes bootstrap made in v5.0 was the removal of jQuery as their dependency. Because of this several plugins were no longer compatible with bootstrap 5, including the bootstrap-multiselect package that we currently use in FFCS to filter slots. There are ways to tweak the package to work with bootstrap 5, but it didn't seem like a good idea to me, there was always a possibility it would break in future and the last time an update was released for this was two years ago, so it doesn't look like it's being maintained anymore.

I googled around and found another plugin that does the same thing with even better design, bootstrap-select. This package's latest version is currently v1.13.18, not compatible with bootstrap 5 just yet, however it is still being maintained and I think v1.14.0 will be out soon since v1.14.0-beta2 is currently the latest pre-release and is compatible with bootstrap 5. I tried using the beta version, while it worked there were also several other issues that still needed to be fixed so I decided not to use the pre-release.

For now, I've imported the bootstrap 4 JS files via CDN only for the latest stable bootstrap-select package and it works great. I've added a comment that this has to be removed when a stable version of the package with bootstrap 5 compatibility is out.

This is what it looks like.

[therealsujitk]

@vatz88, I just noticed a small bug that I believe was caused by my previous PR (#174). When a course is double clicked from the course list, it doesn't load it into the course input. I'll fix this in the clean-up PR.

[vatz88]

Thanks for noticing! Which clean up PR though? We can merge the PR with only this fix, it doesn't have to wait for other changes since it's already broken on prod

[therealsujitk]

Alright, I'll open a PR tomorrow fixing this issue. The FFCS Redesign PR (#179) mostly only changes the design and almost none of the business logic. So it shouldn't break anything and can be merged. However there are a lot of unused resources and redundant code that need to be removed. So for that I'll create another PR cleaning up everything, moving functions and rules to different files to keep things organized.

[therealsujitk]

@vatz88, for the timetable screenshot, i'm working on improving it. Users will be able to download a pdf file containing the timetable and the course list. The best part, It'll be text embedded in the pdf and not images, so the quality would be pretty good.

There's still a lot of adjusting to to do, but hopefully it comes out to what I have in mind.

[vatz88]

Awesome! We should keep it as an additional option, can't remove image option because images are easier to share (on whatsapp with friends) and quickly view

[therealsujitk]

This is what the PDF file will look like. If you'd like me to add some more things, let me know.

---

I originally thought this could fully replace the images, since the scroll to top was a hack and could fail at any time and I assumed sending PDF files were just as easy if not better than the images. Plus through WhatsApp or other platforms, images always get compressed. If you'd rather have both options I'll have to tweak the UI a bit, and if that's the case, it'll be better to do that after #179 is reviewed and merged. I'll have to take a look at why the screenshots get cut when not at the top of the page.

[therealsujitk]

I've cleaned up the SCSS files and removed everything that's redundant (which was quite a bit). I've separated out colours into a separate file like this.

$colors-light: (
    primary: #9c27b0,
    primary-dark: #812092,
    primary-light: #f8ceff,
    ...
);

This way the colours can be modified easily and the pull request for dark theme will be reviewing a single file.

[therealsujitk]

@vatz88, there's an event to prevent the context menu from opening in buttons and the timetable. Why was this added?

$('.btn, #timetable').contextmenu(function() {
    return false;
});

[vatz88]

@vatz88, there's an event to prevent the context menu from opening in buttons and the timetable. Why was this added?

I don't remember, we can remove it

[therealsujitk]

@vatz88, I've improved downloading images (which I'll be adding to the cleanup PR). The hack of scrolling to the top is no longer needed. This is how it looks.