Skip to content

Conversation

@aldenlamp
Copy link
Contributor

Overview

This PR updates the detail page to display hours conforming to the updated model system and data. The biggest change (other than progressively cleaning up the code) is to allow the hours' display to have a variable number of lines (for days with multiple hours sections).

Changes Made

Comments

This is progressively cleaning up/updating the code related to the detail page so I've commented out a lot of the underdeveloped functionality with TODOs here and there. Most of this is seen in the GymDetailView commenting out different cell types.

Models

I've updated the model controller from FitnessCenterManager to GymManager. Maybe a mistake on my part in initial design, but this class should manage both gyms and fitness centers (potentially classes as well in the future in which case updates to come?). All the previous functionality of fitness centers is still there now with added gym functionality.

The Gym class has been reworked to represent the updated database's representation of a gym.

General cleanup

There seemed to be an overwhelming amount of code in the controllers that seemed like it would be more suited for the models. For example, a lot of code for calculating hours, the order of the hour cells in the disclosure view, the strings etc. Those have been moved to put a lot more of the core functionality in the models. This feels to me like a better distribution of responsibility in the architecture.

Another small note, I am progressively moving files around to make the file hierarchy more natural. For example, I've added groupings in the Controller folder for the Home Screen and for the Detail Screen. Similarly, I'm hoping to fade out the organize files by type (group for cells, group for header, group for footer) in favor of grouping by related functionality (group for detail screen controllers, group for detail screen views including cells and footers with subgroups for each related type). In my opinion, this feels a lot more intuitive and scalable than before.

Test Coverage

Tested using different gyms to see the different hour table view heights. I haven't tested some significant edge cases such as if hours data is missing for a day, but this should never happen from the database side?

Next Steps (delete if not applicable)

Waiting on designs (and backend??), but the next step is to implement the views for the other parts of the detail page.

Copy link
Contributor

@tiffany-pann tiffany-pann left a comment

Choose a reason for hiding this comment

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

Looks good to me! Nice work in cleaning up the codebase, factoring things out, and for the very helpful PR description again :]

Copy link
Contributor

@emarc0314 emarc0314 left a comment

Choose a reason for hiding this comment

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

LGTM

@aldenlamp aldenlamp merged commit c852991 into dev/detail-page Nov 1, 2023
@aldenlamp aldenlamp deleted the alden/detail-page branch November 1, 2023 21:26
@aldenlamp aldenlamp mentioned this pull request Nov 1, 2023
aldenlamp added a commit that referenced this pull request Nov 1, 2023
* Update gym hours in gym detail page (#271)

* Update gym hours in gym detail page

* Fix linebreaks

* Implement Tabbed Controller (#272)

* Implement tabbed controller

* Move code to tabbedviewcontroller

* Update styling

* Add capacity to Gym Detail page (#273)

* Add capacity view

* Add linebreak

* Complete primitive detail page (#274)

* Complete primitive detail page

* Address comments
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.

4 participants