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

feat: added punctuality display (#122) #473

Draft
wants to merge 27 commits into
base: main
Choose a base branch
from

Conversation

tobe-official
Copy link
Collaborator

No description provided.

rawi-coding and others added 25 commits December 4, 2024 17:20
# Conflicts:
#	das_client/lib/app/pages/journey/train_journey/widgets/train_journey.dart
#	das_client/lib/model/journey/base_data.dart
#	das_client/lib/model/journey/datatype.dart
#	das_client/lib/model/journey/metadata.dart
#	das_client/lib/sfera/src/mapper/sfera_model_mapper.dart
#	das_client/lib/sfera/src/model/network_specific_parameter.dart
#	das_client/test/sfera/mapper/sfera_mapper_test.dart
#	das_client/test_resources/sp/SFERA_SP_9999_2.xml
#	das_client/test_resources/sp/SFERA_SP_9999_3.xml
#	das_client/test_resources/sp/SFERA_SP_9999_4.xml
# Conflicts:
#	das_client/integration_test/test/train_journey_table_test.dart
#	das_client/lib/app/pages/journey/train_journey/widgets/table/additional_speed_restriction_row.dart
#	das_client/lib/app/pages/journey/train_journey/widgets/table/base_row_builder.dart
#	das_client/lib/app/pages/journey/train_journey/widgets/table/cab_signaling_row.dart
#	das_client/lib/app/pages/journey/train_journey/widgets/table/cells/bracket_station_cell_body.dart
#	das_client/lib/app/pages/journey/train_journey/widgets/table/cells/route_cell_body.dart
#	das_client/lib/app/pages/journey/train_journey/widgets/table/protection_section_row.dart
#	das_client/lib/app/pages/journey/train_journey/widgets/table/service_point_row.dart
#	das_client/lib/app/pages/journey/train_journey/widgets/train_journey.dart
#	das_client/lib/model/journey/cab_signaling.dart
#	das_client/lib/model/journey/track_equipment.dart
…so added Duration into Metadata and changed the UI so that it only shows the delay from the metadata. Still have to check if it is needed to add hours
@tobe-official tobe-official changed the title 122 puenktlichkeit anzeige feat: Added punctuality display (#122) Dec 20, 2024
# Conflicts:
#	das_client/test/sfera/mapper/sfera_mapper_test.dart
@tobe-official tobe-official changed the title feat: Added punctuality display (#122) feat: added punctuality display (#122) Dec 20, 2024
crossAxisAlignment: CrossAxisAlignment.center,
children: [
Text(
'05:43:00',
Copy link
Collaborator

Choose a reason for hiding this comment

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

Das ist nicht in deiner Story drin, aber du könntest hier noch die aktuelle Systemzeit anzeigen.

style: SBBTextStyles.largeBold.copyWith(fontSize: 24.0),
final bloc = context.trainJourneyCubit;

return StreamBuilder<Journey?>(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Der StreamBuilder sollte weiter unten im Widget-Tree verwendet werden. Die Journey brauchst du nur im _punctualityDisplay, deshalb sollte der StreamBuilder auch auf dieser Stufe sein.

Das sollte zwar nicht vorkommen, aber prüfe trotzdem wie das UI aussieht, wenn die Journey nicht oder noch geladen wird. Da du den StreamBuilder verschiebst, musst du das evtl. anders lösen.

image

@@ -150,6 +159,20 @@ class SferaModelMapper {
);
}

static Duration _stringToDuration(String stringToChange) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Ich würde diese Logik gleich im "Domain-Model" Delay führen, z.B. eine toDuration() Methode.


if (delay == null) {
//Todo change the error-code
throw FormatException('Invalid ISO 8601 duration format');
Copy link
Collaborator

Choose a reason for hiding this comment

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

Ich würde hier eine Warnung loggen und null zurückgeben. Besser wir zeigen die Pünktlichkeit nicht an, statt ein Fehler zu werfen.

return Journey(
metadata: Metadata(
nextStop: servicePoints.length > 1 ? servicePoints[1] as ServicePoint : null,
currentPosition: journeyData.first,
additionalSpeedRestrictions: additionalSpeedRestrictions,
routeStart: journeyData.firstOrNull,
routeEnd: journeyData.lastOrNull,
delay: _stringToDuration(relatedTrainInformation == null
? '+PT0M0S' //Base Value for when the information is Null
Copy link
Collaborator

Choose a reason for hiding this comment

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

In diesem Fall wäre es sinnvoller, einfach null den Metadaten zu übergeben.


class TrainLocationInformation extends SferaXmlElement {
static const String elementType = 'TrainLocationInformation';

Copy link
Collaborator

Choose a reason for hiding this comment

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

nitpick: new Line zu viel.

@@ -60,6 +60,7 @@ dependencies:
synchronized: ^3.3.0
# https://pub.dev/packages/flutter_svg
flutter_svg: ^2.0.14
iso_duration: ^0.1.1
Copy link
Collaborator

Choose a reason for hiding this comment

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

nitpick: Damits einheitlich bleibt, noch die URL darüber kopieren wie bei den anderen.

Copy link
Collaborator

@Grodien Grodien Jan 6, 2025

Choose a reason for hiding this comment

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

Sind wir uns sicher dass wir dieses Package verwenden wollen?

  • Last Version 16 Month ago
  • Unverified Publisher
  • 1 Like only

Then again, worst case einfach forken und was fixen? was denkst du @rawi-coding

Copy link
Collaborator

Choose a reason for hiding this comment

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

@Grodien Ich habe das vor den Ferien mit Raphael besprochen. Wir nehmen die Library aber decken unsere Cases mit Unit-Tests ab. Nicht ganz glücklich die Library, sie hat aber zumindest keine Abhänigkeiten. Solang es funktioniert, würde ich sie verwenden.

@@ -68,4 +68,4 @@ SPEC CHECKSUMS:

PODFILE CHECKSUM: d9dad56c0cd0b4fd8b4fe3034a53fd42a0b990f6

COCOAPODS: 1.16.1
COCOAPODS: 1.15.2
Copy link
Collaborator

Choose a reason for hiding this comment

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

cocoapods bei dir noch updaten, dieser change reverten

builder: (context, snapshot) {
if (!snapshot.hasData ||
snapshot.data == null ||
snapshot.data!.metadata.delay == null) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

wenn delay null ist würde ich eher einfach nichts anzeigen statt ein ProgressIndicator

final String minutes = (delay.inMinutes.abs() % 60).toString();
final String seconds = (delay.inSeconds.abs() % 60).toString();
final String hours = delay.inHours.abs().toString();
final String formattedDuration = '${delay.isNegative ? '-' : '+'}$hours:$minutes:${seconds.padLeft(2, '0')}';
Copy link
Collaborator

@Grodien Grodien Jan 6, 2025

Choose a reason for hiding this comment

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

wenn stunden gesetzt sind sollte minuten wohl auch immer zweistellig dargestellt werden? würde sonst komisch aussehen "1:4:53"

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ich habe nachgefragt, es ist gewollt, dass das Format mm:ss bleibt und nicht hh:mm:ss wird. Ich fixe aber noch dass es immer zweistellig ist. 👍

Copy link
Collaborator

@Grodien Grodien left a comment

Choose a reason for hiding this comment

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

paar kleine anmerkungen

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.

3 participants