theme added #659
theme added #659
Conversation
|
🎉 Welcome @Sattar078!
We appreciate your contribution! 🚀 |
WalkthroughAdds a new "Bright" theme and social-profile support (Discord/Twitter/Instagram) across model, controllers, and profile UI, and introduces a standalone Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant BrowserUI as "index.html UI"
participant FirebaseAuth as "Firebase Auth"
participant Firestore as "Firestore"
User->>BrowserUI: open page / interact (toggle theme, switch tabs)
BrowserUI->>FirebaseAuth: initialize / signIn (anon or token)
FirebaseAuth-->>BrowserUI: auth state (user ID)
BrowserUI->>Firestore: fetch friends/requests (using user ID)
Firestore-->>BrowserUI: return lists
BrowserUI->>User: render lists, show actions
User->>BrowserUI: trigger action (Accept/Remove/Invite)
BrowserUI->>Firestore: update documents (accept/remove/invite)
Firestore-->>BrowserUI: update result
BrowserUI->>User: show feedback modal
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
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: 0
🧹 Nitpick comments (1)
lib/themes/theme_list.dart (1)
69-71: Consider removing UI-specific comments.The inline comments reference specific UI elements ("Join Button", "Share Icon") that are implementation details outside this theme configuration. These comments may become stale if the UI structure changes, and the hex color values are already self-documenting.
Apply this diff to simplify:
ThemeModel( name: "Bright", - primaryColor: Color(0xFF4CAF50), // Vibrant green for Join Button + primaryColor: Color(0xFF4CAF50), onPrimaryColor: Colors.white, - secondaryColor: Color(0xFF2196F3), // Clean blue for Share Icon + secondaryColor: Color(0xFF2196F3), onSecondaryColor: Colors.white, surfaceColor: Colors.white, onSurfaceColor: Colors.black, themeMode: ThemeMode.light, ),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pubspec.lockis excluded by!**/*.lock
📒 Files selected for processing (1)
lib/themes/theme_list.dart(1 hunks)
🔇 Additional comments (1)
lib/themes/theme_list.dart (1)
67-76: LGTM! Well-structured theme addition.The new "Bright" theme follows the established pattern consistently and uses appropriate Material Design colors (green primary, blue secondary) for a light theme.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
lib/controllers/user_profile_controller.dart (1)
36-41: Consider adding specific error handling for user document fetch.While the existing try-catch block handles exceptions, consider providing more specific error handling or user feedback when the user document fetch fails. This would improve the user experience if the creator profile is unavailable or the documentId is invalid.
For example, you could add a check before fetching:
try { + if (creatorId.isEmpty) { + log('initializeProfile: creatorId is empty'); + return; + } Document userDocument = await databases.getDocument(Or add more specific error messaging for the getDocument call.
lib/controllers/auth_state_controller.dart (1)
226-228: LGTM! Population logic is consistent with other fields.The social URL fields are populated correctly from the user document, following the same pattern as existing profile fields. The code appropriately handles these as optional values.
For additional robustness, consider validating URL format when these fields are updated in the database (e.g., during profile editing), though this is not critical at the read level.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
lib/controllers/auth_state_controller.dart(2 hunks)lib/controllers/user_profile_controller.dart(2 hunks)lib/models/resonate_user.dart(1 hunks)lib/views/screens/profile_screen.dart(3 hunks)
🔇 Additional comments (4)
lib/controllers/user_profile_controller.dart (1)
24-24: LGTM! Reactive field follows existing patterns.The
searchedUserfield is well-structured, follows the existing reactive pattern usingRx, and appropriately uses a nullable type to represent the absence of data before initialization.lib/controllers/auth_state_controller.dart (1)
55-57: LGTM! Field declarations follow existing patterns.The three new social URL fields are properly declared as
late String?, consistent with other user profile fields in the controller. The nullable type appropriately represents optional data.lib/views/screens/profile_screen.dart (1)
27-28: Dependencies are properly declared in pubspec.yaml.Both
url_launcher(^6.3.2) andfont_awesome_flutter(^10.12.0) are correctly listed in your pubspec.yaml and installed as direct main dependencies. The imports on lines 27-28 are valid.lib/models/resonate_user.dart (1)
17-19: [rewritten comment]
[classification tag]
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
index.html (4)
104-106: Add ARIA labels to dynamic state toggle buttons.The state toggle buttons (lines 104-105) initially lack text content. Add
aria-labelattributes for accessibility before JavaScript populates the text:- <button onclick="toggleState('friends')" id="toggleFriendsBtn" class="px-3 py-1 text-sm rounded-full transition-colors duration-150"></button> - <button onclick="toggleState('requests')" id="toggleRequestsBtn" class="px-3 py-1 text-sm rounded-full transition-colors duration-150"></button> + <button onclick="toggleState('friends')" id="toggleFriendsBtn" aria-label="Toggle friends list state" class="px-3 py-1 text-sm rounded-full transition-colors duration-150"></button> + <button onclick="toggleState('requests')" id="toggleRequestsBtn" aria-label="Toggle requests list state" class="px-3 py-1 text-sm rounded-full transition-colors duration-150"></button>
121-128: Improve modal accessibility with ARIA role and backdrop interaction.The action feedback modal should have proper ARIA attributes and backdrop interaction for better accessibility. Update the modal HTML:
- <div id="action-modal" class="fixed inset-0 bg-gray-900 bg-opacity-50 hidden items-center justify-center p-4 z-50"> - <div class="bg-white dark:bg-gray-700 p-6 rounded-xl shadow-2xl w-full max-w-sm"> + <div id="action-modal" class="fixed inset-0 bg-gray-900 bg-opacity-50 hidden items-center justify-center p-4 z-50" role="alertdialog" aria-labelledby="modal-title" onclick="event.target === this && document.getElementById('action-modal').classList.add('hidden')"> + <div class="bg-white dark:bg-gray-700 p-6 rounded-xl shadow-2xl w-full max-w-sm" onclick="event.stopPropagation()">This enables users to close the modal by clicking the backdrop and improves screen reader support.
322-330: LGTM: Rendering logic is correct. Consider extracting tab class logic for clarity.The render functions work correctly and follow a consistent pattern. The tab button class assignment (lines 328-329) could be simplified with a helper function:
function getTabClasses(isActive) { const activeClasses = 'text-blue-600 dark:text-teal-400 border-b-2 border-blue-600 dark:border-teal-400'; const inactiveClasses = 'text-gray-500 hover:text-gray-700 dark:text-gray-400 dark:hover:text-teal-300 border-b-2 border-transparent hover:border-gray-300 dark:hover:border-gray-600'; return isActive ? activeClasses : inactiveClasses; } function renderTabs() { friendsTab.className = 'tab-button flex-1 py-3 text-center text-sm font-semibold transition-all duration-200 ' + getTabClasses(state.activeTab === 'friends'); requestsTab.className = 'tab-button flex-1 py-3 text-center text-sm font-semibold transition-all duration-200 ' + getTabClasses(state.activeTab === 'requests'); }
209-305: Unsanitized user data in component generators creates XSS vulnerability pattern.The component functions interpolate user data directly into HTML strings and onclick handlers (
${f.name},${r.name}at lines 250, 253, 261, 280, 283). While current hard-coded mock data is safe, this pattern is vulnerable if Firestore integration provides untrusted input. Onclick attribute injection is particularly dangerous—a malicious name could break out of the string and inject JavaScript.Create an HTML escaping utility:
function escapeHtml(text) { const div = document.createElement('div'); div.textContent = text; return div.innerHTML; }Apply to all user data in templates, especially onclick handlers where string context allows attribute escaping.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
index.html(1 hunks)
🔇 Additional comments (5)
index.html (5)
1-8: LGTM: HTML setup and Tailwind integration look good.Standard HTML5 structure and CDN-based Tailwind CSS is appropriate for this standalone single-file client app.
131-147: LGTM: State management approach is appropriate for app scope.The simple object-based state with direct mutation is reasonable for this standalone client app. Hard-coded sample data is fine for demonstration purposes.
157-180: LGTM: Theme toggle and persistence logic is sound.The theme implementation correctly applies the
darkclass to the root element, persists preference to localStorage, and re-renders the UI to update color schemes. SVG icon selection logic is correct.
369-400: LGTM: Event handlers and initialization logic are solid.Theme preference loading correctly prioritizes localStorage over system preference, and the DOMContentLoaded listener ensures proper initialization order. The empty state on page load effectively showcases the UI.
9-41: Fix the dark mode CSS selector syntax.Line 19 uses
.dark body { ... }which targets a<body>element with an ancestor.darkclass. However, thedarkclass is applied to the<html>element (line 170), so this selector won't match. The body background color in dark mode won't be applied.Since dark mode colors are primarily handled via Tailwind's
dark:utility classes on actual elements, this CSS rule may be redundant. Either remove it or correct the selector tohtml.dark body:- /* Dark Mode adjustments */ - .dark body { - background-color: #111827; /* Gray-900 */ - } + /* Dark Mode adjustments */ + html.dark body { + background-color: #111827; /* Gray-900 */ + }Likely an incorrect or invalid review comment.
2 participants
Description
Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.
Fixes # (issue)
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
Please include screenshots below if applicable.
Checklist:
Maintainer Checklist
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.