Skip to content

Conversation

enderv
Copy link
Contributor

@enderv enderv commented Oct 3, 2022

Addresses #245

@netlify
Copy link

netlify bot commented Oct 3, 2022

Deploy Preview for tsml-ui ready!

Name Link
🔨 Latest commit eacf342
🔍 Latest deploy log https://app.netlify.com/sites/tsml-ui/deploys/633e33d7ed6d5a000859cf3c
😎 Deploy Preview https://deploy-preview-259--tsml-ui.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@joshreisner
Copy link
Contributor

thanks! i will check this out with sites currently using TSML UI who are not supplying end times, to see if they are comfortable with this update

changes look great overall, some small bits of feedback:

  • it would be great if you could format with Prettier, looks like at least one of the files would be affected
  • i don't think we need the google sheet change - the purpose of that function is to prepare the end_time for processing in the main load-meeting-data function, so in the HH:MM text format basically
  • in format-ics i think we can just remove the end_time logic now, since meetings with a start will also now have an end

@enderv
Copy link
Contributor Author

enderv commented Oct 3, 2022

got prettier installed and formatted, undid the google change but wasn't sure what to do on "in format-ics i think we can just remove the end_time logic now"

Also thoughts on an issue to add prettier as dev dependency and npm command to run it?

Comment on lines 13 to 16
//need an end time. use default if none specified
if (!meeting.end) {
meeting.end = meeting.start.plus({ hour: 1 });
meeting.end = meeting.start.plus({ minutes: settings.defaults.duration });
}
Copy link
Contributor

Choose a reason for hiding this comment

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

this is what i was referencing. now that we are supplying end times, i think this set of lines can now just be

if (!meeting.start || !meeting.end) return;

@joshreisner
Copy link
Contributor

left a comment on format-ics

sounds great on creating an issue to add prettier as dev dependency and npm command to run it -- i wasn't aware that was possible

@enderv
Copy link
Contributor Author

enderv commented Oct 6, 2022

Oops sorry been swamped last two days got the issue in though. Any other fixes needed after your commit?

@joshreisner
Copy link
Contributor

Hey, no, should be all good now. I just have to give AA SF / Marin a few days to implement end times if they want to. Looking to merge on Sunday / Monday

@joshreisner joshreisner merged commit 9cc7ffb into code4recovery:main Oct 7, 2022
joshreisner added a commit that referenced this pull request Jun 2, 2023
* add a default duration for when end not specified

* add a default duration for when end not specified

* undo google changes

* final tweaks

* fix test

Co-authored-by: Josh Reisner <1551689+joshreisner@users.noreply.github.com>
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