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

Calendar event across days with time, shows end date/time, even if showsEndOnlyWithDuration:false (default) #3598

Open
sdetweil opened this issue Oct 24, 2024 · 3 comments
Assignees

Comments

@sdetweil
Copy link
Collaborator

sdetweil commented Oct 24, 2024

using 2.29
test ics

BEGIN:VCALENDAR
BEGIN:VEVENT
DTSTART:20241026T010000Z
DTEND:20241026T230000Z
DTSTAMP:20241024T153358Z
UID:4maud6s79m41a99pj2g7j5km0a@google.com
CREATED:20241024T153313Z
LAST-MODIFIED:20241024T153330Z
SEQUENCE:0
STATUS:CONFIRMED
SUMMARY:Sleep over at Bobs
TRANSP:OPAQUE
END:VEVENT
END:VCALENDAR

shows end date/time, regardless of showsEndOnlyWithDuration setting

@sdetweil sdetweil added the bug label Oct 24, 2024
@sdetweil
Copy link
Collaborator Author

existiing code in calendar.js tests 2 things at the same time

if(showsEndOnlyWithDuration === true AND startdate===endate){
    do nothing
} else{
   add end display on
}

should be

if( startdate !== endate ){
  if( showsEndOnlyWithDuration === true ){
   add end display on
  }
}

will fix and testcase for both conditions

@sdetweil
Copy link
Collaborator Author

sdetweil commented Oct 24, 2024

SO.. fixing this causes a breaking change, as the setting was effectively ignored before, no testcase to test it

currently default is false.

fixing code breaks a few testcases.. generally shouldn't change testcases.. thats why they are there

change the default to true (same effective behavior as before fix) allows old testcases to continue

this exposes another issue.. we can't allow new config parms without at least 2 testcases.. with and without.. I don't know how we can enforce that in build

currently waiting

@sdetweil
Copy link
Collaborator Author

I think we live with the breaking change.. default is false..
I fixed the testcases...

@sdetweil sdetweil self-assigned this Nov 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant