www
/
www-new
12
3
Fork 5
Code Issues 58 Pull Requests 42 Projects Releases Wiki Activity

[OLD] Add end date and time to events. #434

Closed
snedadah wants to merge 14 commits from events-end-time into main
pull from: events-end-time
merge into: www:main
Conversation 0 Commits 14 Files Changed 549 +685 -574
snedadah commented 2022-05-05 21:41:36 -04:00
Owner
Copy Link

NOTE: This PR was moved to #446 inorder to provide a cleaner git commit history.

Added an end date and time to events.

Usage:
Add an (optional) "endDate" property to an event's markdown file in order to add an end date to that events.

Staging:
https://csclub.uwaterloo.ca/~a3thakra/csc/events-end-time/

Note:
Multiday events are extremly rare and won't happen very often in all likelyhood.

Note2:
"added test events" commit will be reverted before merging the branch, it is primarly there so we can view the events in the staging.

NOTE: This PR was moved to https://git.csclub.uwaterloo.ca/www/www-new/pulls/446 inorder to provide a cleaner git commit history. Added an end date and time to events. Usage: Add an (optional) "endDate" property to an event's markdown file in order to add an end date to that events. Staging: https://csclub.uwaterloo.ca/~a3thakra/csc/events-end-time/ Note: Multiday events are extremly rare and won't happen very often in all likelyhood. Note2: "added test events" commit will be reverted before merging the branch, it is primarly there so we can view the events in the staging.
snedadah added 8 commits 2022-05-05 21:45:25 -04:00
continuous-integration/drone/push Build is passing Details
0f1e8b7e81 Link from [term] to [term]/[event] (#412) 
Closes #189

https://csclub.uwaterloo.ca/~a3thakra/csc/j285he-term-to-term-event/events/

Co-authored-by: Jared He <66887902+jaredjhe@users.noreply.github.com>
Reviewed-on: #412
Reviewed-by: n3parikh <n3parikh@csclub.uwaterloo.ca>
continuous-integration/drone/push Build is passing Details
e74e2279b4 Add Code Party Event (#419) 
Staging: https://csclub.uwaterloo.ca/~a3thakra/csc/amy-events-code-party-w22
Co-authored-by: Amy <a258wang@uwaterloo.ca>
Reviewed-on: #419
Reviewed-by: n3parikh <n3parikh@csclub.uwaterloo.ca>
Co-authored-by: Amy <a258wang@csclub.uwaterloo.ca>
Co-committed-by: Amy <a258wang@csclub.uwaterloo.ca>
continuous-integration/drone/push Build is passing Details
174ddff299 Addec unix101-1 recording to tech talks (#415) 
Added unix 101 event recording

Co-authored-by: shahanneda <shahan.neda@gmail.com>
Reviewed-on: #415
Reviewed-by: j285he <j285he@localhost>
continuous-integration/drone/push Build is passing Details
c13b6a98f9 Added custom 404 page (#410) 
Closes #282

demo: https://csclub.uwaterloo.ca/~a3thakra/csc/404page/404/
Co-authored-by: shahanneda <shahan.neda@gmail.com>
Reviewed-on: #410
Reviewed-by: j285he <j285he@localhost>
Reviewed-by: Amy <a258wang@csclub.uwaterloo.ca>
continuous-integration/drone/push Build is passing Details
672a4ce013 s22-hiring-news (#420) 
Add spring 2022 hiring news

Co-authored-by: Rebecca-Chou <beihaozhou@gmail.com>
Reviewed-on: #420
Reviewed-by: Amy <a258wang@csclub.uwaterloo.ca>
continuous-integration/drone/push Build is passing Details
08405cabee Add EOT event (#421) 
Co-authored-by: Rebecca-Chou <beihaozhou@gmail.com>
Reviewed-on: #421
Reviewed-by: Amy <a258wang@csclub.uwaterloo.ca>
continuous-integration/drone/push Build is passing Details
419a3dd3c6 regen website
continuous-integration/drone/push Build is passing Details
af69b0ab98 Update Get Involved page membership instructions (#429) 
Previously, the instructions for "Online Term" on the Get Involved page would be open by default. This PR changes the instructions for "In Person" to be open by default.

There are also some other small wording changes on the Get Involved page.

Co-authored-by: Amy <a258wang@uwaterloo.ca>
Reviewed-on: #429
Reviewed-by: b38peng <b38peng@uwaterloo.ca>
snedadah requested review from n3parikh 2022-05-05 21:55:52 -04:00
snedadah requested review from j285he 2022-05-05 21:55:52 -04:00
snedadah requested review from a258wang 2022-05-05 21:55:52 -04:00
snedadah requested review from b72zhou 2022-05-05 21:55:52 -04:00
snedadah requested review from e26chiu 2022-05-05 21:55:53 -04:00
a3thakra reviewed 2022-05-05 23:56:55 -04:00
components/EventCard.tsx Outdated
@ -11,6 +11,7 @@ interface EventCardProps {
name: string;
short: string;
date: Date;
endDate?: Date;
a3thakra commented 2022-05-05 23:56:55 -04:00
Owner
Copy Link

naming nit: I would either

  • rename date to startDate, or
  • rename date to start and endDate to end
naming nit: I would either - rename `date` to `startDate`, or - rename `date` to `start` and `endDate` to `end`
a3thakra commented 2022-05-05 23:57:06 -04:00
Owner
Copy Link

same comment but everywhere ^

same comment but everywhere ^
snedadah commented 2022-05-13 19:27:13 -04:00
Author
Owner
Copy Link

The reasoning behind not doing this was to maintain backwards compatibility with the years of previous events.

Since the endDate is optional, I think in this case it makes sense that the start date is just called date, since many times we woulnd't put endDate, and if we do, I think "date" implying its the date, or the start of the event is clear.

The reasoning behind not doing this was to maintain backwards compatibility with the years of previous events. Since the endDate is optional, I think in this case it makes sense that the start date is just called date, since many times we woulnd't put endDate, and if we do, I think "date" implying its the date, or the start of the event is clear.
a258wang commented 2022-05-15 15:38:42 -04:00
Owner
Copy Link

I personally would prefer to rename date to startDate, just for clarity and consistency. Can we update old events with a straightforward find-and-replace, ie. search for the regex ^date: in content/events?

I personally would prefer to rename `date` to `startDate`, just for clarity and consistency. Can we update old events with a straightforward find-and-replace, ie. search for the regex `^date:` in `content/events`?
snedadah commented 2022-05-18 22:02:44 -04:00
Author
Owner
Copy Link

Ok, I'll switch it over to startDate and update the old files.

Ok, I'll switch it over to startDate and update the old files.
a3thakra reviewed 2022-05-06 00:04:11 -04:00
components/EventSetting.tsx Outdated
@ -30,2 +77,4 @@
{dateDisplay}
{separator}
{location}
{separator}
a3thakra commented 2022-05-06 00:04:11 -04:00
Owner
Copy Link

This separator is extra afaik

This separator is extra afaik
snedadah marked this conversation as resolved
a258wang reviewed 2022-05-15 15:49:02 -04:00
components/EventSetting.tsx Outdated
@ -19,3 +20,4 @@
minute: "numeric",
timeZoneName: "short",
});
time = time.substring(0, time.length - 3); // remove the time zone tag (eg PDT) from the start date
a258wang commented 2022-05-15 15:49:02 -04:00
Owner
Copy Link

Could we accomplish the same thing using const time = props.date.toLocalTimeString(...).slice(0, -3) and thereby avoid this reassignment? I personally feel like React encourages a functional programming paradigm, and I feel that doing everything all at once is a little cleaner, but ultimately let's use whatever we think is most readable.

Also, a NIT: perhaps we should rename time to startTime, for clarity?

Could we accomplish the same thing using `const time = props.date.toLocalTimeString(...).slice(0, -3)` and thereby avoid this reassignment? I personally feel like React encourages a functional programming paradigm, and I feel that doing everything all at once is a little cleaner, but ultimately let's use whatever we think is most readable. Also, a NIT: perhaps we should rename `time` to `startTime`, for clarity?
a258wang reviewed 2022-05-15 16:16:34 -04:00
components/EventSetting.tsx Outdated
@ -23,1 +39,4 @@
const separator = <span className={styles.separator}> | </span>;
let dateDisplay;
if (!endDate) {
a258wang commented 2022-05-15 16:16:34 -04:00
Owner
Copy Link

This is an interesting way of handling different event durations! A couple of ideas/suggestions:

  1. I wonder if we could simplify things by displaying...
    • For single-day events (with or without endDate): startDate | startTime | location
    • For multi-day events: startDate - endDate | location
  • My rationale for this is because I think the EventSetting is more like a brief summary of an event, and it can become a little difficult for a reader to parse too many numbers/letters/dashes.
    • For single-day events: I think the start time is much more important than the end time, and users can easily read the event description to find the end time if they're interested.
    • For multi-day events: I think the dates are much more important than the precise start/end time of such events, plus it can get a bit clunky if we want to try to display all this information in the EventSetting. For example, for the upcoming CxC Summit, the start/end time weren't even publicly announced in the marketing blurb.
  1. Have we considered using a ternary operator here, instead of an if statement? The way I see it, if statements are best for conditional logic, whereas ternary operators are best for conditional assignment. This would also allow us to do const dateDisplay = ... instead of let dateDisplay;.
This is an interesting way of handling different event durations! A couple of ideas/suggestions: 1. I wonder if we could simplify things by displaying... - For single-day events (with or without `endDate`): `startDate | startTime | location` - For multi-day events: `startDate - endDate | location` - My rationale for this is because I think the EventSetting is more like a brief summary of an event, and it can become a little difficult for a reader to parse too many numbers/letters/dashes. - For single-day events: I think the start time is much more important than the end time, and users can easily read the event description to find the end time if they're interested. - For multi-day events: I think the dates are much more important than the precise start/end time of such events, plus it can get a bit clunky if we want to try to display all this information in the EventSetting. For example, for the upcoming CxC Summit, the start/end time weren't even publicly announced in the marketing blurb. 2. Have we considered using a ternary operator here, instead of an if statement? The way I see it, if statements are best for conditional logic, whereas ternary operators are best for conditional assignment. This would also allow us to do `const dateDisplay = ...` instead of `let dateDisplay;`.
snedadah commented 2022-05-18 22:06:56 -04:00
Author
Owner
Copy Link

So to confirm, do you suggest that the internal endDate (which would only be the ending time) would not be displayed anywhere on the site for single day events, and instead only in the description provided in the markdown file?

So to confirm, do you suggest that the internal endDate (which would only be the ending time) would not be displayed anywhere on the site for single day events, and instead only in the description provided in the markdown file?
a258wang commented 2022-05-18 22:48:28 -04:00
Owner
Copy Link

Yep, I personally think having the ending time for single-day events only in the event description is okay. My reasoning:

  • The main benefit to having the endDate in the code (even if it's not programmatically being displayed on the website) is that the events.ics calendar will display the event as the correct duration.
  • This will allow for visual consistency with single-day events that don't have endDate specified, as it might look a bit odd if for some events we display 7:00 PM and for others we display 7:00 PM - 9:00 PM. (Though, another way to achieve visual consistency would be to set a default endDate of startDate + 1 hour.)
Yep, I personally think having the ending time for single-day events only in the event description is okay. My reasoning: - The main benefit to having the `endDate` in the code (even if it's not programmatically being displayed on the website) is that the events.ics calendar will display the event as the correct duration. - This will allow for visual consistency with single-day events that don't have `endDate` specified, as it might look a bit odd if for some events we display `7:00 PM` and for others we display `7:00 PM - 9:00 PM`. (Though, another way to achieve visual consistency would be to set a default `endDate` of `startDate + 1 hour`.)
snedadah commented 2022-05-21 02:29:42 -04:00
Author
Owner
Copy Link

I with the points you raise, I ended up implementing it this way in #446.

I with the points you raise, I ended up implementing it this way in #446.
snedadah changed target branch from main to 404page 2022-05-21 00:36:35 -04:00
snedadah changed target branch from 404page to main 2022-05-21 00:36:46 -04:00
snedadah force-pushed events-end-time from 6e2e5a7998 to 8e38007573 2022-05-21 01:36:38 -04:00 Compare
snedadah closed this pull request 2022-05-21 01:42:19 -04:00
snedadah changed title from Add end date and time to events. to [OLD] Add end date and time to events. 2022-05-21 02:01:17 -04:00
All checks were successful
continuous-integration/drone/push Build is passing
Required
Details

Pull request closed

This pull request cannot be reopened because the branch was deleted.
Sign in to join this conversation.
Reviewers
No reviewers
n3parikh
j285he
a258wang
b72zhou
e26chiu
Labels
Clear labels   
a11y

   
Backlog

   
Blocked

   
Bug

   
Content

   
Dependencies

   
Design

   
Feature Request

   
Good First Issue

   
In Progress

   
Performance

   
Priority - High

   
Priority - Low

   
Priority - Medium

   
Untriaged
No Label
a11y
Backlog
Blocked
Bug
Content
Dependencies
Design
Feature Request
Good First Issue
In Progress
Performance
Priority - High
Priority - Low
Priority - Medium
Untriaged
Milestone
Clear milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
No Assignees
3 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: www/www-new#434
No description provided.
Delete Branch "events-end-time"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?