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: evacuation screen and small refactor of file structure in digit… #489

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

tomasz-trela
Copy link
Member

I refactored file structure a little bit. I decided to do this in tab (not in another screen) because when it's look more consistent. I know it's not the best practice in ux, but I think not many people will use this tab (so ui will benefit from this).
Simulator Screenshot - iPhone 16 Plus - 2024-12-14 at 12 56 20
I'm open for any suggestions.

@tomasz-trela tomasz-trela self-assigned this Dec 14, 2024
@tomasz-trela tomasz-trela linked an issue Dec 14, 2024 that may be closed by this pull request

Choose a reason for hiding this comment

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

Copilot reviewed 2 out of 21 changed files in this pull request and generated no comments.

Files not reviewed (19)
  • lib/config/ui_config.dart: Language not supported
  • lib/features/digital_guide_view/data/models/digital_guide_response.dart: Language not supported
  • lib/features/digital_guide_view/data/models/digital_guide_response_extended.dart: Language not supported
  • lib/features/digital_guide_view/data/repository/digital_guide_repository.dart: Language not supported
  • lib/features/digital_guide_view/presentation/digital_guide_view.dart: Language not supported
  • lib/features/digital_guide_view/presentation/widgets/accessibility_button.dart: Language not supported
  • lib/features/digital_guide_view/presentation/widgets/digital_guide_data_source_link.dart: Language not supported
  • lib/features/digital_guide_view/presentation/widgets/digital_guide_features_section.dart: Language not supported
  • lib/features/digital_guide_view/presentation/widgets/digital_guide_go_to_button.dart: Language not supported
  • lib/features/digital_guide_view/presentation/widgets/headlines_section.dart: Language not supported
  • lib/features/digital_guide_view/presentation/widgets/report_change_button.dart: Language not supported
  • lib/features/digital_guide_view/tabs/amenities/presentation/amenities_expansion_tile_content.dart: Language not supported
  • lib/features/digital_guide_view/tabs/entraces/data/models/digital_guide_entrace.dart: Language not supported
  • lib/features/digital_guide_view/tabs/entraces/data/repository/entraces_repository.dart: Language not supported
  • lib/features/digital_guide_view/tabs/evacuation/evacuation_widget.dart: Language not supported
  • lib/features/digital_guide_view/tabs/models/digital_guide_entrace.dart: Language not supported
  • lib/features/digital_guide_view/tabs/surrounding/data/repository/surrounding_repository.dart: Language not supported
  • lib/features/digital_guide_view/tabs/surrounding/presentation/surroundings_expansion_tile_content.dart: Language not supported
  • lib/features/navigator/app_router.dart: Language not supported
@simon-the-shark simon-the-shark added the enhancement New feature or request label Dec 14, 2024
Copy link
Member

@simon-the-shark simon-the-shark left a comment

Choose a reason for hiding this comment

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

Good job Tomuś!

I like the file structure. Only thing is that you have some duplicate trash files in my opinion, DigitalGuideEntrace models are twice in the filesystem and the LocalizationExpansionTileContent inside localization in not in the tabs. Rest look good.

Few notes left, one is a big missing one from the previous @24bartixx big PR - idk how I missed that.

I also propose to create a separate evacutation repo, but not sure if I got everything right (didn't study the responses in very much detail)

Tell me what you think!

@tomasz-trela
Copy link
Member Author

I can't create separate repo for evacuation because evacuation is a property of building( buildings endpoint).

@simon-the-shark
Copy link
Member

I can't create separate repo for evacuation because evacuation is a property of building( buildings endpoint).

Your evacuation repo can use the building data provider:

@riverpod
Future<EvacuationModel> evacuationRepo(Ref ref, int id) async {
  final building = await ref.watch(getDigitalGuideDataProvider(id).future);
  final evacuationMapUrl =
      await ref.watch(getImageUrlProvider(building.evacuationMapId).future);

  // ...
  //...
  //...
  
  return EvacuationModel(
    //...
  );
}

or am i wrong?

@tomasz-trela
Copy link
Member Author

I can't create separate repo for evacuation because evacuation is a property of building( buildings endpoint).

Your evacuation repo can use the building data provider:

@riverpod
Future<EvacuationModel> evacuationRepo(Ref ref, int id) async {
  final building = await ref.watch(getDigitalGuideDataProvider(id).future);
  final evacuationMapUrl =
      await ref.watch(getImageUrlProvider(building.evacuationMapId).future);

  // ...
  //...
  //...
  
  return EvacuationModel(
    //...
  );
}

or am i wrong?

Now I understand

@tomasz-trela
Copy link
Member Author

Done

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feat (digital-guide): add evacuation tile
2 participants