Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Display free rooms based on solely the room scheduling website #22

Merged
merged 8 commits into from
Feb 27, 2020

Conversation

dcarpenter31
Copy link
Contributor

This would be a first step to resolving #8. This displays the rooms available according to the Stevens scheduling website in the popup. A known limitation of this is that it cannot determine if the room has been booked by something like a club- this does not use any data from Virtual EMS. It could also look prettier in the popup, but there are no features with any CSS yet, so a new issue could be opened for that.

@adapap
Copy link
Collaborator

adapap commented Feb 27, 2020

Great implementation of this feature! I agree that this is a very nice starting point -- you managed to scrape the data necessary (we should come to an agreement on what library or libraries to use for HTML scraping in Javascript), as well as implement the display of said data. I believe that as time goes on we will find what works best and standardize our features to use a similar structure, but again, great work!

Copy link
Collaborator

@adapap adapap left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here are some suggestions to improve on the code you already wrote (not what could be added). Approved so we can demo this today!

@@ -0,0 +1,335 @@
import ROOMS from './Rooms';
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would say just to be safe, we would extract the rooms based on what the website presents to us visually (as opposed to hard-coding the room values). This will be easier to do with a web-scraping or HTML-parsing library.

Comment on lines +53 to +58
for (let i = 0; i < ROOMS.length; i++) {
//match all of the tables loaded in the HTML to a the name of a room
//checking by hand, it seems there are only as many tables as there are rooms
//no other tables for decoration or some other data, etc.
roomTableDict[ROOMS[i]] = roomTables[i];
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This construct can usually be replaced with for..of syntax. It's cleaner and usually more intuitive.


chrome.alarms.onAlarm.addListener(alarm => {
if (alarm.name === 'rooms') {
getAvailableRooms(new Date()).then(rooms => {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider using moment.js for anything related to dates and times. It integrates very easily with Javascript Date objects and it's already part of the project dependencies.

@@ -0,0 +1,108 @@
const ROOMS = [
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, scraping this instead of hard-coding values is probably one of the next steps.

@adapap adapap merged commit f789e9b into master Feb 27, 2020
@adapap adapap deleted the free-rooms branch February 27, 2020 23:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants