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

new timetable logic #1850

Open
wants to merge 2 commits into
base: v28.0.00
Choose a base branch
from
Open

new timetable logic #1850

wants to merge 2 commits into from

Conversation

jyyblue
Copy link

@jyyblue jyyblue commented Sep 16, 2024

New Timetable UI and timeslot.

Description

This PR introduces a modern, responsive, timetable that is more flexible than the current one. It also lays the groundwork for future expansion by using new CSS based styling. It also introduces an optional new way of timetabling classes by adding time slots. This allows a very flexible approach to timetabling that generates timetables on-the-fly per user.

  1. Add responsive timetable.
  • Add new js file for responsive timetable:
    -- resources\assets\timetable\util.js
    -- resources\assets\timetable\weekly-schedule.min.js
  • Add new css file for responsive timetable
    -- resources\assets\timetable\timetable.css
  1. Add Terms to Course Add / Edit page.
  2. Add Time Slots to Class.
  3. Add Exceptions to Class.
  4. Change modules/ Timetable / moduleFunctions.php
  • Add timeslot based on Time Slot of Class.

Motivation and Context

Some users find the current current timetable system too rigid. The new system is capable of handling very complex schedules with ease, it is also much easier to set up. The responsive layout makes it much easier to use the timetable on mobile devices.

How Has This Been Tested?

OS: Windows 10
XAMPP : xampp-windows-x64-8.1.10-0-VS16-installer.exe
PHP : PHP 8.1
Database : MariaDB 10.4.25
Development Tool: VS Code

Screenshots

Screenshot_5
Screenshot_6
Screenshot_7
Screenshot_8
Screenshot_9

Remove Timetable day and add timeslot per class.
@SKuipers
Copy link
Member

Thank you @jyyblue for your PR, which looks very useful for schools with less complex timetables. Its been a busy school week but I'm aiming to have time to review the code tomorrow.

@jyyblue
Copy link
Author

jyyblue commented Jan 21, 2025

Are there any update for this PR ?

@SKuipers
Copy link
Member

SKuipers commented Feb 5, 2025

Hi @jyyblue Thank you for your patience with this PR. Part of the reason this has taken so long, aside from ending up very busy on this end, is that this PR makes some very large scale changes to a core part of Gibbon's functionality. The timetable is central to how a school uses Gibbon on a day to day basis, so it is very important to us to ensure that it remains functioning for all use cases. This PR adds new functionality, which is useful for some new use cases, but would need some additional consideration for how it would affect current and future schools using Gibbon

I do appreciate the huge learning curve when getting up and running with Gibbon, so I think you've done an admirable job adding in a new feature set. Generally, with this scale of changes, we'd be happy to discuss things and offer advice as to how to go about beginning making these changes in a way that works well for the core. This helps prevent spending time making changes that might conflict or overlap with ongoing development.

In reviewing the code, the nature of your changes appear to be in a possibly workable state, for development purposes, but they are not necessarily clear and maintainable and ready to be merged. In review, I noted the following concerns:

  • Some vital functions in Timetable/moduleFunctions.php have been duplicated with old versions left in place, with a lot of left over code.
  • Several table names are referenced case insensitive, whereas Gibbon needs to remain case sensitive, as most servers running it have this requirement.
  • The version number does not need incremented for new features created in a development version (v28 or v29). Gibbon's version numbering isn't quite fully semantic, and new versions are only incremented with new releases.
  • The PR adds a large amount of uncommented JS and util functions, but their purpose and how they are used is rather unclear.
  • There is a lot of random code commented out here and there.
  • There is a syntax error in the CHANGEDB entry with the $count variable.
  • There is an unidentified sql.txt file that appears to have been for development purposes.
  • There is an unrelated change to the installer, possibly for development purposes.

I appreciate that many areas of the Gibbon codebase are legacy code, so there is variation in the code standards and the clean coding practices. We do aim to try and ensure any new code added is as clear and maintainable as possible.

We are also always working on refactoring Gibbon, so during v28, we have also made a number of improvements to the UI, that certain aspects of the JS and CSS changes in this PR conflict with. Unfortunately, I appreciate that it wouldn't have been possible to know this without being in contact to learn about the ongoing changes.

I am happy to work together to suggest ways to perhaps break this feature set into some smaller steps that we could add and test in v29, one step at a time, to help ensure things work and fit into the core.

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