search

CRLSCSClub / CRLSTime

A clock for showing the CRLS schedule during the school day
6 stars 10 forks source link

Only refactor code, no optimizations yet (works on 7-year-old browser) #39

Closed psvenk closed 5 years ago

psvenk commented 5 years ago

25 and #30/#32 were previous attempts to refactor and optimize the code which worked in recent versions of Chrome/Chromium and Firefox but which resulted in breaking compatibility with older browsers due to optimizations made (some of which incorporated recently created JavaScript features). This PR refactors the code and does absolutely nothing else, and the resulting code works on an iPhone 3GS running iOS 5 (see screenshots at the bottom).

Note: there are a lot of commits because I am just merging testing into master. Everything before the last few commits has been reverted and is originally from #30. Also, the diff makes it seem like I just made some tweaks to the files and moved some things around. This is because Git is comparing the former schedule.js and draw.js (which are from #25 and are in the directory 71c in the code that I am merging) with the schedule.js and draw.js in the code that I am merging (which are just lines copied and pasted from index.html), and treating the files in the directory 71c as brand-new files (when they are actually just the files formerly in the root directory). I use the term "formerly" a bit loosely here-- "formerly" means "master before merging this pull request".

Screenshots: IMG_0876 IMG_0877 IMG_0878

psvenk commented 5 years ago

By the way, you can test this code at: https://psvenk.github.io/CRLSTime/ (click on testing)

psvenk commented 5 years ago

I am aware that the schedule in use is the late start for conferences— I’ll change it back to normal tonight. If the clock works properly (including on older browsers) now, it should also work properly once the schedule is changed back to normal.

psvenk commented 5 years ago

The code is back to normal. You should be able to test it at: https://psvenk.github.io/CRLSTime/web/testing/

psvenk commented 5 years ago

@dougmcg Now that this code has been tested to work on your phone, would you be willing to kindly give your blessing to allow this pull request to be merged?

psvenk commented 5 years ago

@Derp7777 I saw that you made some changes and then reverted them, but I don't think it's a good idea to push to the testing branch right now when there is an open pull request. I know that I was wrong in opening a pull request from testing to master instead of opening one from a feature branch to master, but the current set-up makes it so that if you add commits to testing, those commits will go into master when this pull request is merged (without necessarily being tested).

psvenk commented 5 years ago

@dougmcg Thank you for merging the pull request.

I have deleted the testing branch because I have realized that my creation of the testing branch was a mistake. In GitHub, the correct way to do pull requests seems to be to create a branch for a specific feature (either on this repository or on a fork) and to make a pull request from that branch to master. This way, the feature branch will only be updated if there is a bug in the code, and other people can have their own pull requests open concurrently (people won't have to wait in line for testing). I am sorry, I created the testing branch because I am a bit new to Git and GitHub, but I have now deleted it.