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

Saturday shows up twice #304

Open
naikymen opened this issue Aug 17, 2023 · 6 comments · May be fixed by #318
Open

Saturday shows up twice #304

naikymen opened this issue Aug 17, 2023 · 6 comments · May be fixed by #318
Labels
bug Something isn't working

Comments

@naikymen
Copy link

naikymen commented Aug 17, 2023

Description of the bug

When polling by week-day instead of specific dates, Saturday shows up twice if selected.

Sample: https://crab.fit/any-day-any-time-by-week-308952

Setup

Captura de Pantalla 2023-08-17 a la(s) 09 18 47

Issue

Captura de Pantalla 2023-08-17 a la(s) 09 20 51

To reproduce

  1. Go to crab.fit
  2. Select "by weekday"
  3. Select all weekdays and all times.
  4. Create
  5. Select any time on saturday

Expected behavior

Saturday should show up once, and selecting times on saturday should select only one saturday.

Additional information

Thanks for the awesome app, we love it.

@naikymen naikymen added the bug Something isn't working label Aug 17, 2023
@WillNilges
Copy link

I've seen this too on a self-hosted dev instance. It might have something to do with whether or not you adjust the time range. It didn't happen when I did 9-5 (the default) but it did when I did 8-8 or 9-8.

@WillNilges
Copy link

I think it has something to do with how the times wrap around (Maybe UTC?) I'm in EST and I noticed that it would act funny when I included 8PM.

image

I also noticed that there was a null between 12-31 and 01-01, which manifests itself here as a gap in the chart.

@WillNilges
Copy link

Reading the code some more, I think that null is a feature. I have a hacky idea for how to fix this: Deduplicate dates by checking if two days are a saturday if isSpecificDates is false.

@WillNilges
Copy link

WillNilges commented Jan 7, 2024

Well, because now it's Sunday, Jan 6, 2024, the bug(s) no longer happens. But, I have been pawing through the code, trying to figure out if there isn't some way to fix this from the beginning, instead of having to modify the code that generates the chart.

So far, I've seen this app work just fine, save for this visual bug, so I'm a bit hesitant to mess with anything too low level, but this appears to be where the dates are chosen when you choose to make a weekly crab fit.

Wait, maybe it's here

@WillNilges
Copy link

Looking at that code, actually, I wonder if the best thing to do might be to find the nearest (probably future but it might not matter) Sunday, and use that instead of Temporal.Now. That might fix it 🤔

@WillNilges
Copy link

WillNilges commented Jan 8, 2024

So as far as I can tell, the logic that actually creates the event is fine. It always generates 7 days. But in the editor, the 8th day shows up in the list of dates. I can't help but think that the dates are stored properly in the backend, and somewhere in the frontend, there's an off-by-one error or timezone nonsense that causes this.

I am like..... 63% sure that the problem is somewhere in convertDatesToTimes.ts

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants