-
Notifications
You must be signed in to change notification settings - Fork 493
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
User schedule #2185
base: master
Are you sure you want to change the base?
User schedule #2185
Changes from 6 commits
055afb1
7cd4cf8
ec2f978
65f3db6
94b5628
eebcf6b
52eb61c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,6 +3,7 @@ | |
class SchedulesController < ApplicationController | ||
load_and_authorize_resource | ||
before_action :respond_to_options | ||
before_action :favourites | ||
load_resource :conference, find_by: :short_title | ||
load_resource :program, through: :conference, singleton: true, except: :index | ||
before_action :load_withdrawn_event_schedules, only: [:show, :events] | ||
|
@@ -58,9 +59,17 @@ def events | |
@events_schedules = @program.selected_event_schedules( | ||
includes: [:room, { event: %i[track event_type speakers submitter] }] | ||
) | ||
|
||
@events_schedules = @events_schedules.select{ |e| e.event.favourite_users.exists?(current_user.id) } if @events_schedules && current_user && @favourites | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This may be a slightly different use case, but I think related enough... I am thinking about creating an |
||
@events_schedules = [] unless @events_schedules | ||
@favourites = params[:favourites] == 'true' | ||
|
||
@unscheduled_events = @program.events.confirmed - @events_schedules.map(&:event) | ||
@unscheduled_events = if @program.selected_schedule | ||
@program.events.confirmed - @events_schedules.map(&:event) | ||
else | ||
@program.events.confirmed | ||
end | ||
@unscheduled_events = @unscheduled_events.select{ |e| e.favourite_users.exists?(current_user.id) } if current_user && @favourites | ||
|
||
day = @conference.current_conference_day | ||
@tag = day.strftime('%Y-%m-%d') if day | ||
|
@@ -72,6 +81,10 @@ def app | |
|
||
private | ||
|
||
def favourites | ||
@favourites = params[:favourites] == 'true' | ||
end | ||
|
||
def respond_to_options | ||
respond_to do |format| | ||
format.html { head :ok } | ||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -4,11 +4,15 @@ | |||||
= image_tag speaker.gravatar_url, :class => "img-circle pull-right all-speaker-pic", | | ||||||
:alt => speaker.name, | | ||||||
:title => speaker.name | | ||||||
|
||||||
%p | ||||||
= canceled_replacement_event_label(event, event_schedule) | ||||||
= replacement_event_notice(event_schedule) | ||||||
|
||||||
- if current_user | ||||||
= link_to('#', onClick: 'starClicked();') do | ||||||
%span#star-events{ class: "fa fa-lg #{ event.favourite_users.exists?(current_user.id) ? 'fa-star' : 'fa-star-o' }", | | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should be a class, not an id? (Since it seems like this partial is reusable) |
||||||
"aria-hidden" => "true", | | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In terms of accessibility, something like this would be provide a better description to anyone with a screen reader.
Suggested change
|
||||||
"data-url" => toggle_favorite_conference_program_proposal_path(@conference.short_title, event.id), | | ||||||
"data-user" => current_user.id } | ||||||
%span.h3 | ||||||
= event.title | ||||||
%br | ||||||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -1,17 +1,22 @@ | ||||||
%td.event{ width: "#{ width * span }%" , | | ||||||
colspan: span, | | ||||||
role: "button" } | ||||||
%a.unstyled-link{href: url_for(conference_program_proposal_path(@conference.short_title, event.id))} | ||||||
%div{ onClick: 'eventClicked(event, this);', "data-url" => "#{url_for(conference_program_proposal_path(@conference.short_title, event.id))}" } | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In terms of accessibility (both screen readers and keyboards), this is better as an |
||||||
%div{ class: "elipsis break-words event-title", | | ||||||
style: "-webkit-line-clamp: #{ event_lines(@rooms) }; height: #{ event_height(@rooms) }px;"} | | ||||||
|
||||||
= canceled_replacement_event_label(event, event_schedule, 'schedule-label') | ||||||
|
||||||
- if current_user | ||||||
= link_to('#', onClick: 'starClicked();') do | ||||||
%span#star{ class: "fa fa-lg #{ event.favourite_users.exists?(current_user.id) ? 'fa-star' : 'fa-star-o' }", | | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same as above, this should be a class, since it appears multiple times on the page. |
||||||
"aria-hidden" =>"true", | | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same as above. :)
Suggested change
|
||||||
"data-url" => toggle_favorite_conference_program_proposal_path(@conference.short_title, event.id), | | ||||||
"data-user" => current_user.id } | ||||||
= event.title | ||||||
|
||||||
- event.speakers_ordered.each do |speaker| | ||||||
= image_tag speaker.gravatar_url, :class => "img-circle pull-right speaker-pic", | | ||||||
:alt => speaker.name, | | ||||||
:title => speaker.name, | | ||||||
:style => "height: #{ speaker_height(@rooms) }px; width: #{ speaker_width(@rooms) }px;" | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,6 +5,13 @@ | |
%h1.text-center | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The above line |
||
Program for | ||
= @conference.title | ||
- if current_user | ||
.row | ||
.col-md-12.text-center | ||
= button_to (@favourites ? 'All events' : 'Only favourites events'), | ||
events_conference_schedule_path(@conference.short_title), | ||
params: { favourites: !@favourites }, | ||
method: :get, class: 'btn btn-success' | ||
.dropdown.program-dropdown | ||
%button{ type: "button", class: "btn btn-default dropdown-toggle", 'data-toggle' => "dropdown" } | ||
Dates | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
class CreateEventsUsers < ActiveRecord::Migration | ||
def change | ||
create_table :events_users, id: false do |t| | ||
t.references :event | ||
t.references :user | ||
end | ||
end | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've finally started working on merging this into my own fork for an event this summer. It seems to me that this allows anyone to toggle anyone else's favorite events. Should it not just use
current_user
? Since you should be logged in and have a cookie?I can't really see a use case as well for someone else (such as an admin) toggling another user's favorites so I think the
favourite_user_id
parameter could probably just be removed, but I might be missing something.