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

Location Checkin, Event population with script, Reformatted Datetime objects, etc. #10

Merged
merged 17 commits into from
Apr 10, 2024

Conversation

Saisriram2003
Copy link
Collaborator

No description provided.

Copy link
Collaborator

@molly-yan molly-yan left a comment

Choose a reason for hiding this comment

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

Good work! Please view the comments for things that need adjustments.

As a reminder, please be careful with committing config files for future development, which might include identifier id/id strings that are unique to your XCode runner.

bu_passport/lib/pages/login_page.dart Show resolved Hide resolved
decoration: BoxDecoration(
borderRadius: BorderRadius.circular(5),
color: _currentPage == index ? Colors.blue : Colors.grey.withOpacity(0.5),
bottom: 10,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Use dynamic measurements, instead of magic numbers

Copy link
Collaborator

Choose a reason for hiding this comment

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

Just made the change + some additional margins for the onboarding page. I think we are good to merge after your approval? The logo images have this weird blue line across it but I think Zihan can upload new logo photos to remove that after

content: Text("Checked in successfully!")));
} else {
ScaffoldMessenger.of(context).showSnackBar(
SnackBar(content: Text("Unable to check in: location too far!")));
Copy link
Collaborator

Choose a reason for hiding this comment

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

  • Is "location too far" the only false case here, or there might be other possible false cases, such as "Location permission not granted"?

final List<Event> upcomingEvents = [];

for (Event event in fetchedEvents) {
// List<String> splittedTime = event.eventStartTime.split(",");
Copy link
Collaborator

Choose a reason for hiding this comment

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

Delete unused code


final response = await http.get(requestUrl);

if (response.statusCode == 200) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Need to include the catch error case

location_room_address = ''

try:
eventRoom = event_detail.find_element(By.XPATH, '//dt[text()="Room:"]/following-sibling::dd').text
Copy link
Collaborator

Choose a reason for hiding this comment

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

It's fine for now, but since we might consider using the ICS file provided by IS&T, this method might need to be replaced later.

Copy link
Collaborator

@molly-yan molly-yan left a comment

Choose a reason for hiding this comment

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

Good to be merged.

/Users/saisriram/.pub-cache/hosted/pub.dev/cloud_firestore-4.15.6/
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please prevent committing config files for future PRs.

@lcfmarco lcfmarco merged commit 5ad57ab into main Apr 10, 2024
@molly-yan molly-yan deleted the location-checkin branch April 29, 2024 01:19
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