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

Feature/library reservations #1073

Open
wants to merge 53 commits into
base: develop
Choose a base branch
from

Conversation

jlcrodrigues
Copy link
Contributor

Duplicate of #658
Fetch and display reserved rooms.

At the time these changes were merged into feature/library-reservations. This should probably be reviewed again though.

Copy link

codecov bot commented Jan 24, 2024

Codecov Report

Merging #1073 (9b0aa46) into develop (94bcf02) will decrease coverage by 0%.
Report is 2 commits behind head on develop.
The diff coverage is 5%.

Additional details and impacted files
@@           Coverage Diff            @@
##           develop   #1073    +/-   ##
========================================
- Coverage       16%     16%    -0%     
========================================
  Files          227     236     +9     
  Lines         6862    7083   +221     
========================================
+ Hits          1073    1084    +11     
- Misses        5789    5999   +210     

uni/lib/view/library/library.dart Outdated Show resolved Hide resolved
uni/lib/view/library/library.dart Outdated Show resolved Hide resolved
uni/lib/view/library/widgets/library_reservations_tab.dart Outdated Show resolved Hide resolved
uni/lib/view/library/widgets/library_reservations_tab.dart Outdated Show resolved Hide resolved
uni/lib/view/library/widgets/reservation_row.dart Outdated Show resolved Hide resolved
uni/lib/view/library/widgets/reservation_row.dart Outdated Show resolved Hide resolved
uni/lib/view/library/widgets/reservation_row.dart Outdated Show resolved Hide resolved
);
}
late TabController tabController;
late List<Tab> tabs;
Copy link
Member

Choose a reason for hiding this comment

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

Why does this need to be late?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Both depend on the context so i initialize them on getBody()

Comment on lines +93 to +100
boxShadow: const [
BoxShadow(
color: Color.fromARGB(0x1c, 0, 0, 0),
blurRadius: 7,
offset: Offset(0, 1),
),
],
),
Copy link
Member

Choose a reason for hiding this comment

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

Do we need to do this manually? Can't we use the GenericCard widget?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That code is already on develop. I just changed the file. Maybe it's better to handle this in a separate issue.

uni/lib/view/library/widgets/library_occupation_tab.dart Outdated Show resolved Hide resolved
uni/lib/view/library/widgets/library_reservations_tab.dart Outdated Show resolved Hide resolved
@jlcrodrigues
Copy link
Contributor Author

According to the new app's structure, I put the reservations card in the faculty page after the calendar.

image

@bdmendes
Copy link
Member

There should only be one library card in the aggregate page, IMO.

@dtpreda
Copy link

dtpreda commented Feb 25, 2024

There should only be one library card in the aggregate page, IMO.

This is a good topic of discussion. I think it's best to keep as is for this release and reiterate on this idea from a UI standpoint.

@bdmendes
Copy link
Member

Let's drop this for this release, then.

@bdmendes bdmendes removed this from the March 2023 Release milestone Feb 26, 2024
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.

4 participants