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

Schedule UI improvements #145

Merged
merged 8 commits into from
Mar 20, 2017
Merged

Schedule UI improvements #145

merged 8 commits into from
Mar 20, 2017

Conversation

pcantrell
Copy link
Contributor

A grab bag:

  • Schedule page now works even if sessions don’t have rooms yet. This is in anticipation of getting the schedule out early this year, and assigning rooms only later.
  • The schedule — and the whole app! — should always show dates in Central Time, regardless of the user’s device’s time zone.
  • Fixed broken CSS/JS includes (probably got hosed during the Rails 5 migration?).
  • Mopped up some layout stuff that got messy with incremental fixes over the years.
  • Fixed broken URL in http → https admin redirect. (Should we force https site-wide?)

@coveralls
Copy link

Coverage Status

Coverage increased (+0.4%) to 56.062% when pulling 3372a92 on schedule-ui-improvements into c714c76 on master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.3%) to 55.982% when pulling 3372a92 on schedule-ui-improvements into c714c76 on master.

@attendance_count ||= attendances.count
end

attr_writer :attendance_count
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be helpful to say that we use this in preload_attendance_counts as an optimization.

@jcoyne
Copy link
Contributor

jcoyne commented Mar 20, 2017

By in large these changes look good. Generally it would be helpful to other maintainers if you would add a bit more method documentation.

Copy link
Member

@caseyhelbling caseyhelbling left a comment

Choose a reason for hiding this comment

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

LGTM

@pcantrell
Copy link
Contributor Author

I added a few light comments. Justin, LMK if there's stuff left that still seems unclear to you.

@pcantrell pcantrell merged commit 739021d into master Mar 20, 2017
@jcoyne
Copy link
Contributor

jcoyne commented Mar 20, 2017

@pcantrell thanks man! 😎

@jcoyne jcoyne deleted the schedule-ui-improvements branch September 26, 2017 21:58
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