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

2031: Combine recurring events native #2495

Conversation

steffenkleinle
Copy link
Member

@steffenkleinle steffenkleinle commented Oct 3, 2023

Short description

This PR combines recurring events such that they only show up once in the list.

Proposed changes

  • Show an icon in the event list whether they are today, recurring or today and recurring
  • Always show the next three dates (if available) and give the option to show the next ten dates by using a collapsible
  • Refactor Collapsible to match web implementation

Side effects

None.

Resolved issues

Fixes #2031.


@steffenkleinle steffenkleinle force-pushed the 2031-combine-recurring-events-native branch from b107346 to 8fc97c7 Compare October 3, 2023 11:08
Copy link
Contributor

@f1sh1918 f1sh1918 left a comment

Choose a reason for hiding this comment

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

When i open events on ios, they are not combined @steffenkleinle
This relies probably on local data i think they were not refetched properly after updating the app. When i delete the app and reinstall it everything works.
Maybe there is some additional cache invalidation needed to get this work

Additionally i think same issues than web. Start and end times are not correct and on smaller devices like iPhone SE it won't fit on a single page

Copy link
Contributor

@LeandraH LeandraH left a comment

Choose a reason for hiding this comment

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

Looks good, I'd add a little bit of space between the dates and the address. Tested in emulated and real iOS.
image

native/src/components/DatesPageDetail.tsx Outdated Show resolved Hide resolved
@steffenkleinle
Copy link
Member Author

When i open events on ios, they are not combined @steffenkleinle This relies probably on local data i think they were not refetched properly after updating the app. When i delete the app and reinstall it everything works. Maybe there is some additional cache invalidation needed to get this work

Should be fixed, database should be invalidated on update.

Additionally i think same issues than web. Start and end times are not correct and on smaller devices like iPhone SE it won't fit on a single page

Using 'D' format for numeric months as well, start and end times should be fixed with #2506.

@f1sh1918
Copy link
Contributor

f1sh1918 commented Oct 9, 2023

When i open events on ios, they are not combined @steffenkleinle This relies probably on local data i think they were not refetched properly after updating the app. When i delete the app and reinstall it everything works. Maybe there is some additional cache invalidation needed to get this work

Should be fixed, database should be invalidated on update.

Additionally i think same issues than web. Start and end times are not correct and on smaller devices like iPhone SE it won't fit on a single page

Using 'D' format for numeric months as well, start and end times should be fixed with #2506.

Hm ok looks like you decided not to use short month format @steffenkleinle ?
I'm fine with this solution.

Copy link
Contributor

@f1sh1918 f1sh1918 left a comment

Choose a reason for hiding this comment

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

Tested on ios

@steffenkleinle
Copy link
Member Author

When i open events on ios, they are not combined @steffenkleinle This relies probably on local data i think they were > > > Additionally i think same issues than web. Start and end times are not correct and on smaller devices like iPhone SE it won't fit on a single page

Using 'D' format for numeric months as well, start and end times should be fixed with #2506.

Hm ok looks like you decided not to use short month format @steffenkleinle ? I'm fine with this solution.

You would rather want abbreviated months you mean? Like Nov. instead of 11? I figured two digits is even shorter than three letters. But I don't really mind.

Copy link
Member

@ztefanie ztefanie left a comment

Choose a reason for hiding this comment

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

Tested on Android, works fine

native/src/utils/DatabaseConnector.ts Show resolved Hide resolved
native/src/components/EventListItem.tsx Show resolved Hide resolved
Copy link
Contributor

@LeandraH LeandraH left a comment

Choose a reason for hiding this comment

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

Looks good, I don't mind the months being shown as numbers instead of short names, I think both are pretty clear since the structure follows the conventions for the phone language.

Edit: tested on emulated iOS

@steffenkleinle steffenkleinle merged commit fcfa8c0 into 2031-combine-recurring-events-web Oct 10, 2023
1 of 2 checks passed
@steffenkleinle steffenkleinle deleted the 2031-combine-recurring-events-native branch October 10, 2023 06:39
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.

IGAPP-1078: Combine recurring events
4 participants