[FOSSOVERFLOW-26] UI of all planned features completed#6
Open
Sri-Varshith wants to merge 7 commits intoOpenLake:mainfrom
Open
[FOSSOVERFLOW-26] UI of all planned features completed#6Sri-Varshith wants to merge 7 commits intoOpenLake:mainfrom
Sri-Varshith wants to merge 7 commits intoOpenLake:mainfrom
Conversation
WalkthroughThis PR introduces a new OnlineMeet feature by adding a Meeting model, a dedicated page with scheduled/attended meeting tabs and a scheduling interface, a reusable MeetingCard widget, integrating url_launcher and intl dependencies, updating the homepage navigation, and changing the dark theme accent color from blue to green. Changes
Sequence DiagramsequenceDiagram
actor User
participant HomePage
participant OnlineMeetPage
participant MeetingCard
participant Meeting
User->>HomePage: Tap "1:1 Online Meet"
HomePage->>OnlineMeetPage: Navigate to OnlineMeetPage
OnlineMeetPage->>OnlineMeetPage: Load initial _allMeetings (4 seeded)
OnlineMeetPage->>MeetingCard: Render Scheduled tab with meetings
User->>OnlineMeetPage: Tap FloatingActionButton
OnlineMeetPage->>OnlineMeetPage: Show scheduling bottom sheet
User->>OnlineMeetPage: Select date/time, enter title & notes
User->>OnlineMeetPage: Tap "Schedule" button
OnlineMeetPage->>Meeting: Create new Meeting instance
OnlineMeetPage->>OnlineMeetPage: Append to _allMeetings, rebuild
OnlineMeetPage->>MeetingCard: Update displayed tabs
MeetingCard->>User: Display updated meetings with formatted date
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
flutter_app/lib/pages/online_meet_page.dart (1)
263-265: Keep title-driven rebuilds local to the bottom sheet.At Line [263]-Line [265], every title keystroke triggers full page
setState(). This is avoidable overhead; keep this reactive state inside the sheet (StatefulBuilder/ValueListenableBuilder) instead of rebuilding tabs and lists.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@flutter_app/lib/pages/online_meet_page.dart` around lines 263 - 265, The current _titleController.addListener calls the page-level setState(), causing the whole OnlineMeetPage to rebuild on each keystroke; instead remove that global listener and move the reactive UI into the bottom sheet: when showing the sheet that uses _titleController, wrap the sheet content in a StatefulBuilder or a ValueListenableBuilder (listening to _titleController) and update only the sheet UI there (call setState provided by StatefulBuilder or rely on ValueListenableBuilder rebuilds) so keystrokes only rebuild the bottom-sheet widgets, not the entire page.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@flutter_app/lib/pages/online_meet_page.dart`:
- Around line 115-123: When updating _selectedDateTime inside the bottom sheet
(the setSheetState block that constructs a DateTime from date and time),
validate the composed DateTime against DateTime.now() and prevent selecting a
past datetime: compute the candidate DateTime from the selected date and time,
and if candidate.isBefore(DateTime.now()) either clamp it to DateTime.now() or
show a validation error and do not assign to _selectedDateTime; apply the same
check for the other identical update site (the second location where
_selectedDateTime is set) so meetings for today cannot be scheduled in the past.
- Around line 40-41: The _scheduled and _attended getters rely on
Meeting.isAttended (which uses DateTime.now()) and can drift between itemCount
and itemBuilder; to fix, compute a stable snapshot at build time: capture a
single DateTime now (or compute and store List<Meeting> scheduledSnapshot and
attendedSnapshot) at the start of the widget's build method (or before the
ListView.builder is constructed) and use those snapshots for both itemCount and
itemBuilder instead of the getters, making sure to reference the unique symbols
_scheduled, _attended and Meeting.isAttended when converting the logic so the
lists remain consistent during the frame.
---
Nitpick comments:
In `@flutter_app/lib/pages/online_meet_page.dart`:
- Around line 263-265: The current _titleController.addListener calls the
page-level setState(), causing the whole OnlineMeetPage to rebuild on each
keystroke; instead remove that global listener and move the reactive UI into the
bottom sheet: when showing the sheet that uses _titleController, wrap the sheet
content in a StatefulBuilder or a ValueListenableBuilder (listening to
_titleController) and update only the sheet UI there (call setState provided by
StatefulBuilder or rely on ValueListenableBuilder rebuilds) so keystrokes only
rebuild the bottom-sheet widgets, not the entire page.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7cbc329d-ee13-41a8-aeaa-38e7c4f0e2cd
⛔ Files ignored due to path filters (1)
flutter_app/pubspec.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
flutter_app/lib/core/theme/dark_theme.dartflutter_app/lib/models/meeting_model.dartflutter_app/lib/pages/homepage.dartflutter_app/lib/pages/online_meet_page.dartflutter_app/lib/widgets/meeting_card.dartflutter_app/pubspec.yaml
Comment on lines
+40
to
+41
| List<Meeting> get _scheduled => _allMeetings.where((m) => !m.isAttended).toList(); | ||
| List<Meeting> get _attended => _allMeetings.where((m) => m.isAttended).toList(); |
There was a problem hiding this comment.
Stabilize scheduled/attended snapshots per build to avoid list/index drift.
At Line [40]-Line [41], these getters depend on DateTime.now() transitively (Meeting.isAttended). In ListView.builder, itemCount and itemBuilder can observe different snapshots, causing edge-case out-of-range access around time boundaries.
💡 Proposed fix
- List<Meeting> get _scheduled => _allMeetings.where((m) => !m.isAttended).toList();
- List<Meeting> get _attended => _allMeetings.where((m) => m.isAttended).toList();
+ ({List<Meeting> scheduled, List<Meeting> attended}) _partitionMeetings(DateTime now) {
+ final scheduled = <Meeting>[];
+ final attended = <Meeting>[];
+ for (final meeting in _allMeetings) {
+ if (meeting.scheduledAt.isBefore(now)) {
+ attended.add(meeting);
+ } else {
+ scheduled.add(meeting);
+ }
+ }
+ return (scheduled: scheduled, attended: attended);
+ }
@@
Widget build(BuildContext context) {
+ final now = DateTime.now();
+ final partitions = _partitionMeetings(now);
+ final scheduled = partitions.scheduled;
+ final attended = partitions.attended;
return SafeArea(
@@
- _scheduled.isEmpty
+ scheduled.isEmpty
@@
- itemCount: _scheduled.length,
+ itemCount: scheduled.length,
itemBuilder: (context, index) => MeetingCard(
- meeting: _scheduled[index],
+ meeting: scheduled[index],
@@
- _attended.isEmpty
+ attended.isEmpty
@@
- itemCount: _attended.length,
+ itemCount: attended.length,
itemBuilder: (context, index) => MeetingCard(
- meeting: _attended[index],
+ meeting: attended[index],Also applies to: 315-343
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@flutter_app/lib/pages/online_meet_page.dart` around lines 40 - 41, The
_scheduled and _attended getters rely on Meeting.isAttended (which uses
DateTime.now()) and can drift between itemCount and itemBuilder; to fix, compute
a stable snapshot at build time: capture a single DateTime now (or compute and
store List<Meeting> scheduledSnapshot and attendedSnapshot) at the start of the
widget's build method (or before the ListView.builder is constructed) and use
those snapshots for both itemCount and itemBuilder instead of the getters,
making sure to reference the unique symbols _scheduled, _attended and
Meeting.isAttended when converting the logic so the lists remain consistent
during the frame.
Comment on lines
+115
to
+123
| setSheetState(() { | ||
| _selectedDateTime = DateTime( | ||
| date.year, | ||
| date.month, | ||
| date.day, | ||
| time.hour, | ||
| time.minute, | ||
| ); | ||
| }); |
There was a problem hiding this comment.
Block scheduling in the past for today’s date.
At Line [115]-Line [123], a past time can still be selected for the current day. Those meetings can instantly fall into “Attended,” which breaks the scheduling intent.
💡 Proposed fix
setSheetState(() {
- _selectedDateTime = DateTime(
+ final picked = DateTime(
date.year,
date.month,
date.day,
time.hour,
time.minute,
);
+ _selectedDateTime = picked.isAfter(DateTime.now()) ? picked : null;
});
+ if (_selectedDateTime == null && mounted) {
+ ScaffoldMessenger.of(context).showSnackBar(
+ const SnackBar(content: Text('Please choose a future time')),
+ );
+ }Also applies to: 224-239
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@flutter_app/lib/pages/online_meet_page.dart` around lines 115 - 123, When
updating _selectedDateTime inside the bottom sheet (the setSheetState block that
constructs a DateTime from date and time), validate the composed DateTime
against DateTime.now() and prevent selecting a past datetime: compute the
candidate DateTime from the selected date and time, and if
candidate.isBefore(DateTime.now()) either clamp it to DateTime.now() or show a
validation error and do not assign to _selectedDateTime; apply the same check
for the other identical update site (the second location where _selectedDateTime
is set) so meetings for today cannot be scheduled in the past.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The final features with complete UI:
Summary by CodeRabbit
New Features
Style
Removed Features