Redesigned the Admin page with proper functionality#49
Open
siddhigupta075 wants to merge 2 commits into
Open
Conversation
WalkthroughThis PR redesigns the Admin page with enhanced validation logic and updated UI styling. Changes include implementing proper view binding with cleanup, adding email format and designation validation, introducing new gradient drawable resources, and comprehensively updating the layout structure with refined styling for cards, form inputs, and action buttons. Changes
Sequence DiagramsequenceDiagram
actor User
participant Fragment as AdminFragment
participant UI as UI Elements
participant ViewModel as ViewModel
participant Backend as Backend/Database
User->>Fragment: Click Add Button
Fragment->>Fragment: validateAndAdd()
Fragment->>UI: Check email field
UI-->>Fragment: Email value
Fragment->>UI: Check designation field
UI-->>Fragment: Designation value
alt Validation Fails
Fragment->>UI: Set error on email/designation
UI-->>Fragment: Error displayed
else Validation Passes
Fragment->>UI: Disable Add button
Fragment->>UI: Update button text to "Adding..."
Fragment->>UI: Show progress dialog
Fragment->>ViewModel: addToMessCommittee(email, designation)
ViewModel->>Backend: Submit data
Backend-->>ViewModel: Success/Response
ViewModel-->>Fragment: Result
Fragment->>UI: Dismiss progress dialog
Fragment->>UI: Re-enable Add button
Fragment->>UI: Restore button text
Fragment->>UI: Show success toast
Fragment->>UI: Clear email input
Fragment->>UI: Clear designation input
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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: 3
🧹 Nitpick comments (1)
app/src/main/res/drawable/baseline_attach_email_24.xml (1)
2-2: Removeapp:tintfrom<path>element; tinting is only supported on the<vector>root.The
app:tintattribute on individual<path>elements is not part of the VectorDrawable specification and will have no effect. VectorDrawable only supports tinting viaandroid:tinton the<vector>root element. Path elements should useandroid:fillColororandroid:strokeColorinstead. Removing the unused namespace and attribute clarifies intent and reduces confusion.Proposed cleanup
<vector xmlns:android="http://schemas.android.com/apk/res/android" - xmlns:app="http://schemas.android.com/apk/res-auto" android:width="24dp" android:height="24dp" android:tint="@color/food" android:viewportWidth="24" android:viewportHeight="24"> @@ <path android:fillColor="@android:color/white" - app:tint="@color/food" android:pathData="M21,14v4c0,1.1 -0.9,2 -2,2s-2,-0.9 -2,-2v-4.5c0,-0.28 0.22,-0.5 0.5,-0.5s0.5,0.22 0.5,0.5V18h2v-4.5c0,-1.38 -1.12,-2.5 -2.5,-2.5S15,12.12 15,13.5V18c0,2.21 1.79,4 4,4s4,-1.79 4,-4v-4H21z" /> </vector>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/res/drawable/baseline_attach_email_24.xml` at line 2, The XML has an invalid app:tint on a <path> element and an unnecessary xmlns:app declaration; remove the app:tint attribute from the path(s) and delete the unused xmlns:app namespace on the root <vector> (tinting is only supported via android:tint on the <vector> root). If you intended to change path color, set android:fillColor or android:strokeColor on the affected <path> elements instead, or move any global tint to the <vector> root using android:tint.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@app/src/main/java/com/theayushyadav11/MessEase/ui/NavigationDrawers/Fragments/AdminFragment.kt`:
- Around line 78-89: The async callbacks passed to viewModel.addToMessCommittee
(and the similar callback at lines 94-100) access binding and requireContext()
after async work, which can crash if the Fragment view is destroyed; update the
callback bodies to first check the Fragment/view lifecycle (e.g.,
viewLifecycleOwner.lifecycle.currentState.isAtLeast(Lifecycle.State.STARTED) or
isAdded() && view != null) before touching binding or calling requireContext(),
and bail out silently if the view is gone—ensure all UI updates
(mess.pbDismiss(), binding.btnAdd.*, mess.toast(...), clearing fields) are
guarded by that check or run via view?.let { ... } so no UI is accessed after
destruction.
- Around line 50-61: Before running the validation when-block in AdminFragment
(the block checking email and designation using android.util.Patterns), clear
any existing error messages on the involved TextInputLayouts first; e.g., set
binding.tilEmail.error = null and binding.tilspin.error = null (and any other
tilX used for inputs) immediately before the when { ... } so stale errors are
removed and only current validation failures are shown.
In `@app/src/main/res/layout/fragment_admin.xml`:
- Around line 23-34: The ImageView with id ivHeaderIcon is missing accessibility
semantics; update the element for accessibility by adding an appropriate
android:contentDescription—use a meaningful string resource (e.g.,
`@string/header_icon_description`) if the icon conveys information, or set
android:contentDescription="@null" (and optionally
android:importantForAccessibility="no") if it's purely decorative; ensure you
edit the ImageView with id ivHeaderIcon to include this attribute.
---
Nitpick comments:
In `@app/src/main/res/drawable/baseline_attach_email_24.xml`:
- Line 2: The XML has an invalid app:tint on a <path> element and an unnecessary
xmlns:app declaration; remove the app:tint attribute from the path(s) and delete
the unused xmlns:app namespace on the root <vector> (tinting is only supported
via android:tint on the <vector> root). If you intended to change path color,
set android:fillColor or android:strokeColor on the affected <path> elements
instead, or move any global tint to the <vector> root using android:tint.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9e8999f5-513f-4715-811e-4b0dc9c7f54d
📒 Files selected for processing (7)
app/src/main/java/com/theayushyadav11/MessEase/ui/NavigationDrawers/Fragments/AdminFragment.ktapp/src/main/res/drawable/baseline_attach_email_24.xmlapp/src/main/res/drawable/baseline_person_24.xmlapp/src/main/res/drawable/bg_card_glass_inner.xmlapp/src/main/res/drawable/bg_gradient_full.xmlapp/src/main/res/drawable/bg_icon_soft.xmlapp/src/main/res/layout/fragment_admin.xml
Comment on lines
+50
to
+61
| when { | ||
| email.isEmpty() -> { | ||
| binding.tilEmail.error = "Email required" | ||
| } | ||
|
|
||
| fun add() { | ||
| if (binding.etEmail.text.toString().isNotEmpty()) { | ||
| mess.addPb("Adding to Mess Committee") | ||
| viewModel.addToMessCommittee( | ||
| binding.etEmail.text.toString(), | ||
| binding.spinnerAutoComplete.text.toString() | ||
| ) | ||
| { | ||
| mess.pbDismiss() | ||
| mess.toast(it) | ||
| !android.util.Patterns.EMAIL_ADDRESS.matcher(email).matches() -> { | ||
| binding.tilEmail.error = "Invalid email" | ||
| } | ||
| } else { | ||
| mess.toast("Please enter email") | ||
|
|
||
| designation.isEmpty() -> { | ||
| binding.tilspin.error = "Select designation" | ||
| } |
There was a problem hiding this comment.
Clear field errors before validation branches to avoid stale messages.
If one field becomes valid and another fails, the old error can remain visible.
Small validation cleanup
private fun validateAndAdd() {
val email = binding.etEmail.text.toString().trim()
val designation = binding.spinnerAutoComplete.text.toString().trim()
+ binding.tilEmail.error = null
+ binding.tilspin.error = null
+
when {
email.isEmpty() -> {
binding.tilEmail.error = "Email required"
}
!android.util.Patterns.EMAIL_ADDRESS.matcher(email).matches() -> {
binding.tilEmail.error = "Invalid email"
}
designation.isEmpty() -> {
binding.tilspin.error = "Select designation"
}
else -> {
- binding.tilEmail.error = null
- binding.tilspin.error = null
add(email, designation)
}
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@app/src/main/java/com/theayushyadav11/MessEase/ui/NavigationDrawers/Fragments/AdminFragment.kt`
around lines 50 - 61, Before running the validation when-block in AdminFragment
(the block checking email and designation using android.util.Patterns), clear
any existing error messages on the involved TextInputLayouts first; e.g., set
binding.tilEmail.error = null and binding.tilspin.error = null (and any other
tilX used for inputs) immediately before the when { ... } so stale errors are
removed and only current validation failures are shown.
Comment on lines
+78
to
89
| viewModel.addToMessCommittee(email, designation) { | ||
| mess.pbDismiss() | ||
|
|
||
| binding.btnAdd.isEnabled = true | ||
| binding.btnAdd.text = "Add to Committee" | ||
|
|
||
| mess.toast(it) | ||
|
|
||
| // Clear fields after success | ||
| binding.etEmail.text?.clear() | ||
| binding.spinnerAutoComplete.text?.clear() | ||
| } |
There was a problem hiding this comment.
Guard async callbacks against Fragment view destruction.
These callbacks use binding/requireContext() after async work. If the view is destroyed first, this can crash.
Safer callback handling
private fun add(email: String, designation: String) {
binding.btnAdd.isEnabled = false
binding.btnAdd.text = "Adding..."
mess.addPb("Adding to Mess Committee")
viewModel.addToMessCommittee(email, designation) {
+ val b = _binding ?: return@addToMessCommittee
mess.pbDismiss()
- binding.btnAdd.isEnabled = true
- binding.btnAdd.text = "Add to Committee"
+ b.btnAdd.isEnabled = true
+ b.btnAdd.text = "Add to Committee"
mess.toast(it)
- binding.etEmail.text?.clear()
- binding.spinnerAutoComplete.text?.clear()
+ b.etEmail.text?.clear()
+ b.spinnerAutoComplete.text?.clear()
}
}
private fun setAdapter() {
mess.getLists("${DESIGNATION}s") {
+ val b = _binding ?: return@getLists
+ val ctx = context ?: return@getLists
val adapter = ArrayAdapter(
- requireContext(),
+ ctx,
android.R.layout.simple_dropdown_item_1line,
it
)
- binding.spinnerAutoComplete.setAdapter(adapter)
+ b.spinnerAutoComplete.setAdapter(adapter)
}
}Also applies to: 94-100
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@app/src/main/java/com/theayushyadav11/MessEase/ui/NavigationDrawers/Fragments/AdminFragment.kt`
around lines 78 - 89, The async callbacks passed to viewModel.addToMessCommittee
(and the similar callback at lines 94-100) access binding and requireContext()
after async work, which can crash if the Fragment view is destroyed; update the
callback bodies to first check the Fragment/view lifecycle (e.g.,
viewLifecycleOwner.lifecycle.currentState.isAtLeast(Lifecycle.State.STARTED) or
isAdded() && view != null) before touching binding or calling requireContext(),
and bail out silently if the view is gone—ensure all UI updates
(mess.pbDismiss(), binding.btnAdd.*, mess.toast(...), clearing fields) are
guarded by that check or run via view?.let { ... } so no UI is accessed after
destruction.
Comment on lines
23
to
+34
| <ImageView | ||
| android:id="@+id/ivHeaderIcon" | ||
| android:layout_width="60dp" | ||
| android:layout_height="60dp" | ||
| android:layout_marginStart="25dp" | ||
| android:layout_marginTop="32dp" | ||
| android:src="@drawable/logo" | ||
| android:contentDescription="Mess committee icon" | ||
| app:tint="@color/food" | ||
| android:background="@drawable/bg_icon_soft" | ||
| android:padding="14dp" | ||
| app:layout_constraintStart_toStartOf="parent" | ||
| app:layout_constraintTop_toTopOf="parent" | ||
| app:tint="@color/food" /> | ||
| app:layout_constraintTop_toTopOf="parent"/> |
There was a problem hiding this comment.
Add explicit accessibility semantics for the header icon.
The ImageView lacks contentDescription. If decorative, set android:contentDescription="@null" (and optionally importantForAccessibility="no"); otherwise provide a meaningful string resource.
Suggested fix
<ImageView
android:id="@+id/ivHeaderIcon"
android:layout_width="60dp"
android:layout_height="60dp"
@@
android:src="@drawable/logo"
+ android:contentDescription="@null"
app:tint="@color/food"
android:background="@drawable/bg_icon_soft"
android:padding="14dp"
app:layout_constraintStart_toStartOf="parent"
app:layout_constraintTop_toTopOf="parent"/>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/main/res/layout/fragment_admin.xml` around lines 23 - 34, The
ImageView with id ivHeaderIcon is missing accessibility semantics; update the
element for accessibility by adding an appropriate
android:contentDescription—use a meaningful string resource (e.g.,
`@string/header_icon_description`) if the icon conveys information, or set
android:contentDescription="@null" (and optionally
android:importantForAccessibility="no") if it's purely decorative; ensure you
edit the ImageView with id ivHeaderIcon to include this attribute.
Collaborator
|
@siddhigupta075 Please make the required changes 1. Unsafe
|
1 task
|
🔗 @siddhigupta075 opened PR #50 for this issue and has been auto-assigned. |
1 task
|
🔗 PR #51 by @siddhigupta075 is now linked to this assignment. The deadline timer is paused while the PR is open. |
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
2 participants
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.
Resolves #24
Description
UI Changes:
Code Changes;
Screenshots:
Checkout
Summary by CodeRabbit
New Features
Style