Enhance UI/UX of Application cards#2079
Conversation
WalkthroughThe changes in this pull request involve updates to both CSS and JavaScript files related to application cards in the admin interface. The CSS modifications enhance the layout, hover effects, and visibility of elements within the application cards. The JavaScript changes improve the rendering of application cards, including the management of template controls and user feedback for search results. Collectively, these updates aim to refine the visual structure and interactivity of the application cards. Changes
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![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
🧹 Outside diff range and nitpick comments (2)
css/admin/applications.css (1)
Line range hint
201-216: Consider adding intermediate breakpoint for better responsive behaviorThe current responsive implementation is good, but there might be a significant jump in card sizes between 1200px (2 columns) and 600px (1 column).
Consider adding an intermediate breakpoint:
/* Add this between the existing breakpoints */ @media only screen and (max-width: 900px) { .frm-application-card { grid-column: span 8/span 8; /* 1.5 cards per row */ } }js/admin/applications.js (1)
298-305: LGTM! Consider adding ARIA attributes for the "NEW" pill.The changes improve the visual hierarchy and user experience by adding clear indicators for new items with smooth fade-in animations.
Consider adding
role="status"andaria-labelto the "NEW" pill for better screen reader support:- titleWrapper.appendChild( span({ className: 'frm-new-pill frm-meta-tag frm-fadein', text: __( 'NEW', 'formidable' ) }) ); + titleWrapper.appendChild( span({ + className: 'frm-new-pill frm-meta-tag frm-fadein', + text: __( 'NEW', 'formidable' ), + role: 'status', + 'aria-label': __( 'New template', 'formidable' ) + }) );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
css/admin/applications.css(2 hunks)js/admin/applications.js(2 hunks)
🔇 Additional comments (2)
css/admin/applications.css (1)
70-70: LGTM: Good improvement for layout consistency
Adding a minimum height to card titles prevents layout shifts and ensures visual consistency across all application cards.
js/admin/applications.js (1)
372-372: LGTM! Good use of consistent animations.
The addition of the frm-fadein class maintains visual consistency with other animated elements while preserving proper button styling and accessibility attributes.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2079 +/- ##
============================================
- Coverage 27.18% 27.17% -0.02%
- Complexity 8111 8114 +3
============================================
Files 127 127
Lines 26970 26980 +10
============================================
- Hits 7333 7332 -1
- Misses 19637 19648 +11 ☔ View full report in Codecov by Sentry. |
lauramekaj1
left a comment
lauramekaj1
left a comment
There was a problem hiding this comment.
@AbdiTolesa I tested this and verified it's working as expected. Thank you!
truongwp
left a comment
truongwp
left a comment
There was a problem hiding this comment.
This looks good. Thanks @AbdiTolesa!
Crabcyborg
left a comment
Crabcyborg
left a comment
There was a problem hiding this comment.
Thanks @AbdiTolesa @truongwp and @lauramekaj1 for working on this!
I think this looks good to merge.
🚀
There was a problem hiding this comment.
@AbdiTolesa I noticed a bug with this update.
The custom applications no longer have a delete icon.
This results in JS errors from Pro as well.
…://github.com/Strategy11/formidable-forms into issue-5328-enhance_ui_ux_of_application_cards
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
js/admin/applications.js (1)
298-309: LGTM! Consider adding ARIA attributes for accessibility.The implementation correctly handles the placement of controls and the NEW badge, aligning with the PR objectives. The conditional placement ensures proper inline alignment with the title.
Consider adding ARIA attributes to improve accessibility:
- titleWrapper.appendChild( span({ className: 'frm-new-pill frm-meta-tag frm-fadein', text: __( 'NEW', 'formidable' ) }) ); + titleWrapper.appendChild( span({ + className: 'frm-new-pill frm-meta-tag frm-fadein', + text: __( 'NEW', 'formidable' ), + 'aria-label': __( 'New application template', 'formidable' ) + }) );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
js/admin/applications.js(2 hunks)
🔇 Additional comments (1)
js/admin/applications.js (1)
377-377: LGTM! Button styling aligns with design requirements.
The button styling with frm-button-secondary frm-button-sm class ensures proper inline alignment, and the frm-fadein class provides a consistent animation effect matching the NEW badge.
Crabcyborg
left a comment
Crabcyborg
left a comment
There was a problem hiding this comment.
Thanks for updating that @AbdiTolesa!
Some Cypress tests are failing but it looks like it's happening in master as well. It looks related to changes on formidableforms.com and not related to this PR.
I'll go ahead and merge this.
🚀
4 participants
Fix https://github.com/Strategy11/formidable-pro/issues/5328
Design by Razvan: https://github.com/Strategy11/formidable-pro/issues/5328#issuecomment-2332233644
Test procedure
...but the buttons are always appear inline with it.