Skip to content
This repository has been archived by the owner on Feb 16, 2024. It is now read-only.

[Benchmarks] Add performance benchmarks (1/2) #20

Merged
merged 34 commits into from Mar 25, 2020
Merged

[Benchmarks] Add performance benchmarks (1/2) #20

merged 34 commits into from Mar 25, 2020

Conversation

guidezpl
Copy link
Member

@guidezpl guidezpl commented Mar 24, 2020

This PR contains

Preparation for the perf transitions test

  • Introduction of GalleryDemoCategory (study, material, cupertino, other)
    • Studies are now considered demos
    • Names, display text of demo and categories are centralised in demos.dart
  • Demos are obtained by passing in a GalleryLocalizations object instead of a BuildContext
  • Add describe getter to GalleryDemo to uniquely identify demos with the format 'demoName@demoCategory' (e.g. "Buttons@material")
  • Add ValueKeys for integration tests to identify category headers, demo lists, and demos
  • Create test_driver/transitions_perf.dart, which launches the app in English and provides uniquely identifiable demo names to transitions_perf_test.dart

Misc

  • Update flutter_localized_locales to 1.1.0 (which has a simpler loading model)
  • Fix bug where demo categories on mobile were collapsing themselves when scrolled out of view
  • Miscellaneous Podfile (iOS & macOS) updates
  • Add Flutter Driver to pubspec

Future work

In a separate PR test_driver/transitions_perf_test.dart

Copy link
Contributor

@clocksmith clocksmith left a comment

Choose a reason for hiding this comment

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

LGTM, but wait for one more

lib/data/demos.dart Show resolved Hide resolved
lib/data/demos.dart Show resolved Hide resolved

Map<Symbol, GalleryDemo> studies(GalleryLocalizations localizations) {
return <Symbol, GalleryDemo>{
#shrine: GalleryDemo(
Copy link
Member

Choose a reason for hiding this comment

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

What is Symbol?

Copy link
Member Author

Choose a reason for hiding this comment

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

I thought it was the Dart type for symbols but it's to be used for methods and members instead (https://api.flutter.dev/flutter/dart-core/Symbol/Symbol.html). I'll replace with strings

@@ -11,7 +11,7 @@ import 'package:gallery/layout/text_scale.dart';
import 'package:gallery/l10n/gallery_localizations.dart';
import 'package:gallery/studies/fortnightly/shared.dart';

const fortnightlyTitle = 'Fortnightly';
const _fortnightlyTitle = 'Fortnightly';
Copy link
Member

Choose a reason for hiding this comment

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

Should this be removed and use the value from demos.dart instead?

Copy link
Member Author

Choose a reason for hiding this comment

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

It'd be a bit cumbersome to obtain so I'll leave it as is

@guidezpl guidezpl merged commit a679fa8 into master Mar 25, 2020
@guidezpl guidezpl deleted the benchmarks branch March 25, 2020 12:22
DanTup pushed a commit to DanTup/flutter_gallery that referenced this pull request Jun 30, 2020
* Create transitions_perf.dart

* Add flutter_driver and test dependencies

* Pass localizations instead of BuildContext to demos

Also add categories to demos and fix naming conflicts

* Create WIP transitions_perf_test.dart

* Updates

* Update transitions_perf.dart

* Update pubspec.lock

* Update iOS Podfile

* Update project.pbxproj

* Rename GalleryDemoCategory.reference to other

* Allow extension methods by bumping up sdk

* Update macOS Podfile

* Obtain nativeLocaleNames directly

* Specify category for demos instead of title

* Add extension to GalleryDemoCategory to get name and display title

* Add back and list value keys

* Obtain demo descriptions cleanly

* Add value keys to demos

* studies returns a map and is used

* Add more ValueKeys

* Rename reference to other demos

* Fix categories collapsing themselves

* Delete transitions_perf_test.dart

* Edit copyright

* Use allGalleryDemos

* Update demos.dart

* Update pubspec

* Name extension for it to be available globally

* Use strings instead of symbols for study demos

* Create pubspec.lock

Co-authored-by: guidezpl <guidezpl@users.noreply.github.com>
Former-commit-id: a679fa8
@guidezpl guidezpl changed the title Add performance benchmarks (1/2) [Benchmarks] Add performance benchmarks (1/2) Aug 21, 2020
return LinkedHashMap<String, GalleryDemo>.fromIterable(
materialDemos(context) + cupertinoDemos(context) + referenceDemos(context),
allGalleryDemos(localizations),
key: (dynamic demo) => demo.slug as String,

Choose a reason for hiding this comment

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

@guidezpl I noticed in passing that this change looks like it adds a bunch of entries to the resulting map all with a null key. That is, allGalleryDemos adds in the list of GalleryDemo entries produced from studies, each of which has null as the value of demo.slug. The resulting map will have only a single entry for all of the studies entries, using null as the key. Is this intentional? Or have I somehow missed something here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Studies were implemented first, and then demos added the concept of slugs, so I think that's why it's the way it is. AFAIK studies don't actually use slugToDemo

shaheer-ahmed-dev pushed a commit to shaheer-ahmed-dev/gallery that referenced this pull request May 18, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants