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

Adding czech locale #140

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,15 @@
| ![](https://github.com/JonasWanke/timetable/raw/main/doc/demo-callbacks.webp?raw=true) | ![](https://github.com/JonasWanke/timetable/raw/main/doc/demo-visibleDateRange.webp?raw=true) |

- [Available Layouts](#available-layouts)
- [`MultiDateTimetable`](#multidatetimetable)
- [`RecurringMultiDateTimetable`](#recurringmultidatetimetable)
- [`CompactMonthTimetable`](#compactmonthtimetable)
- [Getting started](#getting-started)
- [0. General Information](#0-general-information)
- [1. Define your `Event`s](#1-define-your-events)
- [2. Create a `DateController` (optional)](#2-create-a-datecontroller-optional)
- [3. Create a `TimeController` (optional)](#3-create-a-timecontroller-optional)
- [4. Create your Timetable](#4-create-your-timetable)
- [4. Create your Timetable widget](#4-create-your-timetable-widget)
- [Theming](#theming)
- [Advanced Features](#advanced-features)
- [Drag and Drop](#drag-and-drop)
Expand Down Expand Up @@ -46,7 +49,6 @@ A Timetable widget that displays [`MonthWidget`]s in a page view.
| :------------------------------------------------------------------------------------------------------------: | :-----------------------------------------------------------------------------------------------------------: |
| ![](https://github.com/JonasWanke/timetable/raw/main/doc/screenshot-CompactMonthTimetable-light.webp?raw=true) | ![](https://github.com/JonasWanke/timetable/raw/main/doc/screenshot-CompactMonthTimetable-dark.webp?raw=true) |


## Getting started

### 0. General Information
Expand All @@ -61,7 +63,7 @@ Some date/time-related parameters also have special suffixes:
- `timeOfDay`: A `Duration` between zero and 24 hours.
- `dayOfWeek`: An `int` between one and seven ([`DateTime.monday`](https://api.flutter.dev/flutter/dart-core/DateTime/monday-constant.html) through [`DateTime.sunday`](https://api.flutter.dev/flutter/dart-core/DateTime/sunday-constant.html)).

Timetable currently offers localizations for Chinese, English, French, German, Hungarian, Italian, Japanese, Portuguese, and Spanish.
Timetable currently offers localizations for Czech, Chinese, English, French, German, Hungarian, Italian, Japanese, Portuguese, and Spanish.
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
Timetable currently offers localizations for Czech, Chinese, English, French, German, Hungarian, Italian, Japanese, Portuguese, and Spanish.
Timetable currently offers localizations for Chinese, Czech, English, French, German, Hungarian, Italian, Japanese, Portuguese, and Spanish.

Even if you're just supporting English in your app, you have to add Timetable's localization delegate to your `MaterialApp`/`CupertinoApp`/`WidgetsApp`:

```dart
Expand Down
1 change: 1 addition & 0 deletions example/lib/utils.dart
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import 'package:timetable/timetable.dart';

final _mediaOverrideState = ValueNotifier(MediaOverrideState());
final _supportedLocales = [
const Locale('cs'),
const Locale('de'),
const Locale('en'),
const Locale('es'),
Expand Down
13 changes: 9 additions & 4 deletions lib/src/components/multi_date_content.dart
Original file line number Diff line number Diff line change
Expand Up @@ -324,10 +324,15 @@ class _PartDayDraggableEventState extends State<PartDayDraggableEvent> {
return;
}

final adjustedOffset = _pointerToWidgetTopCenter(_lastOffset!);
final geometry = _findGeometry(context, adjustedOffset);
widget.onDragCanceled?.call(geometry.key, _wasMoved);
_resetState();
if (mounted) {
final adjustedOffset = _pointerToWidgetTopCenter(_lastOffset!);
final geometry = _findGeometry(context, adjustedOffset);
widget.onDragCanceled?.call(geometry.key, _wasMoved);
_resetState();
} else {
widget.onDragCanceled?.call(null, _wasMoved);
_resetState();
}
Comment on lines +330 to +338
Copy link
Owner

Choose a reason for hiding this comment

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

What problem were you seeing with the previous code?

With the new code, if this widget is created as PartDayDraggableEvent.forGeometryKeys(…), the call to onDragCanceled will throw an exception since it ensures that the geometry key is not null

Copy link
Author

@Musta-Pollo Musta-Pollo May 2, 2023

Choose a reason for hiding this comment

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

Thank you for taking the time to review the code. I apologize for the confusion caused by the incorrect order. I have not realized that the letter "Ch" is specific to my language, Czech.

I have implemented the drag-and-drop functionality in my app, using the example app as a guide. However, there have been instances where the PartDayDraggableEvent becomes unmounted, and the onDragCanceled function crashes due to the unmounted state at _pointerToWidgetTopCenter(_lastOffset!). As a result, the logic to remove the event overlay is not executed.

To address this issue, I have made a change that ensures the onDragCanceled function is called even after the widget is mounted, which removes the overlay in my app.

But I see that it conflicts with other functionality. What do you think I should do?

Copy link
Author

Choose a reason for hiding this comment

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

A stacktrace:

Fatal Exception: io.flutter.plugins.firebase.crashlytics.FlutterError: Null check operator used on a null value. Error thrown Instance of 'ErrorDescription'.
       at State.context(framework.dart:954)
       at _PartDayDraggableEventState._findRenderBox(multi_date_content.dart:274)
       at _PartDayDraggableEventState._pointerToWidgetTopCenter(multi_date_content.dart:334)
       at _PartDayDraggableEventState._onDragCanceled(multi_date_content.dart:327)
       at AsyncValueX.maybeWhen.<fn>(common.dart:624)
       at _DraggableState._startDrag.<fn>(drag_target.dart:525)
       at _DragAvatar.finishDrag(drag_target.dart:897)
       at _DragAvatar.end(drag_target.dart:801)
       at MultiDragPointerState._up(multidrag.dart:158)
       at MultiDragGestureRecognizer._handleEvent(multidrag.dart:266)
       at PointerRouter._dispatch(pointer_router.dart:98)
       at PointerRouter._dispatchEventToRoutes.<fn>(pointer_router.dart:143)
       at _LinkedHashMapMixin.forEach(dart:collection)
       at PointerRouter._dispatchEventToRoutes(pointer_router.dart:141)
       at PointerRouter.route(pointer_router.dart:127)
       at GestureBinding.handleEvent(binding.dart:460)
       at GestureBinding.dispatchEvent(binding.dart:440)
       at RendererBinding.dispatchEvent(binding.dart:336)
       at GestureBinding._handlePointerEventImmediately(binding.dart:395)
       at GestureBinding.handlePointerEvent(binding.dart:357)
       at GestureBinding._flushPointerEventQueue(binding.dart:314)
       at GestureBinding._handlePointerDataPacket(binding.dart:295)```

Copy link
Owner

Choose a reason for hiding this comment

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

No worries, that's an interesting property of the Czech alphabet!

Okay, I see the problem. Maybe we should just drop the geometry key parameter of the onDragCanceled callback? I don't know where it would be useful… That would also mean that the three different typedefs for onDragCanceled could be merged into just one taking a bool wasMoved.

Alternatively, we could save the geometry key from the last onDragUpdate, but that doesn't work if the pointer didn't move or the corresponding geometry got disposed of already. Hence, I lean towards the first option

Copy link
Author

Choose a reason for hiding this comment

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

Thanks :)!

If you think the argument is not helpful, I agree with the first option as it is cleaner.

Copy link
Author

@Musta-Pollo Musta-Pollo May 2, 2023

Choose a reason for hiding this comment

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

Would you like me to do it? Or should I create a new pull request with correctly added Czech locales and leave this to you?

I am currently not dependent upon this change as I use the fork.

Copy link
Owner

Choose a reason for hiding this comment

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

Sorry for the delay! I'll leave that up to you – go ahead if you want to, otherwise, I'll implement that change before the next release :)

}

Offset _pointerToWidgetTopCenter(Offset offset) {
Expand Down
22 changes: 22 additions & 0 deletions lib/src/localization.dart
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import 'week.dart';
///
/// Supported [Locale.languageCode]s:
///
/// * `cs` – Czech
/// * `de` – German
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
/// * `de` – German

/// * `de` – German
/// * `en` – English
/// * `es` – Spanish
Expand Down Expand Up @@ -89,6 +91,8 @@ class TimetableLocalizationsDelegate
return const TimetableLocalizationDe();
case 'en':
return const TimetableLocalizationEn();
case 'cs':
return const TimetableLocalizationCs();
Comment on lines +94 to +95
Copy link
Owner

Choose a reason for hiding this comment

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

Please move this case above 'de' to keep them ordered alphabetically as well

case 'es':
return const TimetableLocalizationEs();
case 'fr':
Expand Down Expand Up @@ -218,6 +222,24 @@ class TimetableLocalizationEn extends TimetableLocalizations {
'Week ${week.weekOfYear}, ${week.weekBasedYear}';
}

class TimetableLocalizationCs extends TimetableLocalizations {
const TimetableLocalizationCs();

@override
List<String> weekLabels(Week week) {
return [
weekOfYear(week),
'Týden ${week.weekOfYear}',
'T ${week.weekOfYear}',
'${week.weekOfYear}',
];
}

@override
String weekOfYear(Week week) =>
'Týden ${week.weekOfYear}, ${week.weekBasedYear}';
}
Comment on lines +225 to +241
Copy link
Owner

Choose a reason for hiding this comment

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

Please move this class above TimetableLocalizationDe to keep them ordered alphabetically


class TimetableLocalizationEs extends TimetableLocalizations {
const TimetableLocalizationEs();

Expand Down