Skip to content

Conversation

@parthshindee
Copy link
Contributor

Summary

This PR addresses performance issues related to lazy loading, stateless widgets, and uncached images (Issue #2068, #2069, #2070).

Changelog

[General] [Change] - Stateful Widget to Stateless where possible
[General] [Add] - Lazy loading to all list like components
[General] [Add] - Caching images

Test Plan

There should be no functional changes in the mobile app. Everything should remain the same. This PR is only intended to improve performance significantly.

klortiz13 and others added 30 commits September 14, 2024 16:28
…e-implementation

implemented cached network image
@morebytes
Copy link
Contributor

@parthshindee @klortiz13 Thank you for the great work guys. However, I'm seeing some inconsistencies with the way that functional methods are being used on data. Before we approve this PR, let's all meet to go over this. This is a good case study for the code style guide project.

}

double fontSizeForTablet() {
double fontSizeForTablet(BuildContext context) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Functions that use BuildContext do not need to pass it as a parameter. Use the context instance variable from the class itself.

}
// RegExp multiPager = RegExp(r' \(\d+/\d+\)$');
// Filter the models and create a list of only the valid ones
List<Widget> locationsList = data
Copy link
Contributor

Choose a reason for hiding this comment

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

I can't tell if this is proper usage of functional-style iterator methods. Let's try to rework this one.

Copy link
Contributor

Choose a reason for hiding this comment

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

@parthshindee let's meet on this one individually to see the best solution. Possible precedent case for the official Campus Mobile Style Guide

final placeholderPhotoUrl = dotenv.get('PLACEHOLDER_PERSON_PHOTO');
bool isValidId = false;
class EmployeeIdCard extends StatelessWidget {
final String cardId = "employee_id";
Copy link
Contributor

Choose a reason for hiding this comment

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

Re-evaluate if this and others like it can be made into const variables.

titleText: CardTitleConstants.titleMap[cardId],
errorText: Provider.of<EmployeeIdDataProvider>(context).error,
child: () => isValidId
child: () => (employeeModel != null &&
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider putting this into a static private method unless it is only used in this function just once.

DateTime? lastUpdated, String? nextDayWithClasses, BuildContext context) {
try {
// Flatten the data into a single list of ListTile widgets
List<Widget> listToReturn = [];
Copy link
Contributor

Choose a reason for hiding this comment

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

This can be converted into functional-style methods.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Performance Enhancements: Widgets Performance Enhancements: Lazy Loading Performance Enhancements: Images and Assets

4 participants