-
Notifications
You must be signed in to change notification settings - Fork 215
feat: add loading state UI and enhance empty state illustrations in `… #2963
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
base: develop
Are you sure you want to change the base?
Conversation
…Status` page - Introduced a loader with custom SVGs for better user feedback during status checks. - Improved empty state illustration with smaller SVG and updated layout for clarity. - Simplified error handling in API call by removing unused parameter.
WalkthroughThe Status component now short-circuits rendering when loading to show a dedicated loading UI. The no-data UI and SVGs were redesigned and typography adjusted. A Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant StatusComponent as Status.tsx
participant DataLayer as API/Data Hook
Note over StatusComponent: Mount / Render
User->>StatusComponent: open status page
StatusComponent->>DataLayer: fetch/check dokan status
DataLayer-->>StatusComponent: loading=true (initial)
alt loading
StatusComponent-->>User: render LoadingUI ("Checking your Dokan Status", spinner)
else loaded
DataLayer-->>StatusComponent: loading=false, data or empty
alt has data
StatusComponent-->>User: render Status Table (`dokan-status-table`)
else no data
StatusComponent-->>User: render NoData UI (centered SVG, "Your Dokan is up-to-date.")
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/Status/Status.tsx (2)
134-231: LGTM! Consider extracting SVGs for maintainability.The loading state implementation is solid—it provides clear user feedback with appropriate visual elements and internationalized text. The early return pattern improves readability.
For better maintainability, consider extracting the inline SVGs into separate components:
// icons/DokanLogoIcon.tsx export const DokanLogoIcon = () => ( <svg width="62" height="62" viewBox="0 0 62 62" fill="none" xmlns="http://www.w3.org/2000/svg"> {/* SVG content */} </svg> ); // icons/SpinnerIcon.tsx export const SpinnerIcon = () => ( <svg className="w-10 h-10 rounded-full animate-spin" viewBox="0 0 27 27" fill="none" xmlns="http://www.w3.org/2000/svg"> {/* SVG content */} </svg> );This would reduce file length and make the icons reusable across the codebase.
274-375: LGTM! Minor accessibility enhancement suggested.The updated empty state UI is well-structured with improved centering and a more personalized message. The SVG illustration and layout changes align with the PR objectives.
Consider adding an accessible label to the SVG for screen readers:
<svg width="146" height="99" viewBox="0 0 146 99" fill="none" xmlns="http://www.w3.org/2000/svg" className="mb-4" + role="img" + aria-label={ __( 'Dokan status illustration', 'dokan-lite' ) } >Note: This empty state will also be shown when the API call fails (see previous comment about error handling).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/Status/Status.tsx(3 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-19T06:19:41.376Z
Learnt from: mrabbani
Repo: getdokan/dokan PR: 2891
File: src/admin/dashboard/pages/vendors-single/InformationTabs/WithdrawTab.tsx:58-59
Timestamp: 2025-09-19T06:19:41.376Z
Learning: The Dokan codebase uses 'dokan-lite' as the text domain for WordPress internationalization (__() function calls).
Applied to files:
src/Status/Status.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: e2e tests (2, 3)
- GitHub Check: e2e tests (3, 3)
- GitHub Check: api tests (1, 1)
- GitHub Check: e2e tests (1, 3)
| .catch( () => { | ||
| setLoading( false ); | ||
| } ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add error logging and user feedback for failed status checks.
The simplified error handler doesn't log errors or inform users when the API call fails. This means users will see the "up-to-date" message (lines 370-371) even when the status check actually failed, which is misleading.
Consider adding error logging and state tracking:
+ const [ error, setError ] = useState< string | null >( null );
+
useEffect( () => {
apiFetch< Array< StatusElement > >( {
path: 'dokan/v1/admin/dashboard/status',
} )
.then( ( data ) => {
setAllSettings( data );
setLoading( false );
} )
- .catch( () => {
+ .catch( ( err ) => {
+ console.error( 'Failed to fetch Dokan status:', err );
+ setError( __( 'Failed to load status. Please try again.', 'dokan-lite' ) );
setLoading( false );
} );
}, [] );Then display the error state in the render section instead of the success message when error is set.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/Status/Status.tsx around lines 46 to 48, the catch block swallows errors
and only calls setLoading(false); update it to log the caught error
(console.error or the app logger), set an error state (e.g., setError(error
message or object) and setLoading(false)), and ensure the render uses that error
state to show an error message/UI instead of the success "up-to-date" text when
error is set; keep existing success flow when no error.
- Added `dokan-status-table` class to improve scoping and avoid global table style conflicts. - Updated Tailwind overrides to exclude `.dokan-status-table` from default resets.
…Status` page
All Submissions:
Changes proposed in this Pull Request:
Related Pull Request(s)
Closes
How to test the changes in this Pull Request:
Changelog entry
No change log needed
Before Changes
Describe the issue before changes with screenshots(s).
After Changes
Describe the issue after changes with screenshot(s).
Feature Video (optional)
Link of detailed video if this PR is for a feature.
PR Self Review Checklist:
FOR PR REVIEWER ONLY:
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.