Enhance: Admin settings page responsive #2475
Conversation
WalkthroughThe changes include the introduction of a new Vue.js component named Changes
Assessment against linked issues
Suggested labels
Suggested reviewers
Poem
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: 0
🧹 Outside diff range and nitpick comments (6)
src/admin/pages/Settings.vue (5)
18-18: Fix typo in class name from "nab-section" to "nav-section"The class name "nab-section" appears to be a typo. It should probably be "nav-section" to maintain consistency with other navigation elements.
Apply this diff to correct the class name:
-<div class="nab-section block"> +<div class="nav-section block">
47-56: Enhance accessibility by adding an aria-label to the search inputTo improve accessibility for screen reader users, consider adding an
aria-labelto the search input element. This provides a textual description of the input's purpose.Apply this diff to add the
aria-label:<input type="text" id="dokan-admin-search" class="w-full px-3 py-2 border-none focus:outline-none focus:ring-0 bg-transparent" :placeholder="__('Search e.g. vendor', 'dokan-lite')" v-model="searchText" @input="searchInSettings" ref="searchInSettings" + :aria-label="__('Search Settings', 'dokan-lite')" />
205-221: Consider renaming 'MobileSettingsList.vue' to 'MobileSettingsDrawer.vue' for clarityThe
MobileSettingsDrawercomponent is exported fromMobileSettingsList.vue. Renaming the file toMobileSettingsDrawer.vuewould improve consistency and make the codebase easier to navigate.Remove unused import of 'MobileSettingsList' component
The
MobileSettingsListcomponent is imported but not used in the code. Removing this unused import will clean up the code.Apply this diff to remove the unused import:
-import MobileSettingsList from "admin/components/Settings/MobileSettingsList.vue"; let Loading = dokan_get_lib('Loading'); let AdminNotice = dokan_get_lib('AdminNotice'); import Fields from "admin/components/Fields.vue" import SettingsBanner from "admin/components/SettingsBanner.vue"; import UpgradeBanner from "admin/components/UpgradeBanner.vue"; import MobileSettingsDrawer from "admin/components/Settings/MobileSettingsList.vue"; export default { name: 'Settings', components: { MobileSettingsDrawer, - MobileSettingsList, Fields, Loading, SettingsBanner, UpgradeBanner, AdminNotice, },
1242-1301: Remove commented-out code to clean up the stylesheetThere's a block of commented-out code from lines 1242 to 1301. Removing unused code enhances readability and maintainability.
Apply this diff to remove the unnecessary code:
-//@media only screen and (max-width: 430px) { -// .dokan-settings-wrap { -// .nav-tab-wrapper { -// -// .nav-tab { -// padding-left: 10px !important; -// -// img { -// margin: 3px 8px 0px 4px; -// } -// -// .nav-content { -// .nav-title { -// font-size: 7px; -// } -// -// .nav-description { -// font-size: 5px !important; -// } -// } -// } -// -// .nav-tab-active { -// &:before { -// width: 2px !important; -// } -// } -// } -// -// .metabox-holder { -// width: 100%; -// -// .settings-header { -// display: block; -// -// .settings-content { -// .settings-title, -// .settings-description { -// padding-left: 0; -// } -// } -// -// .settings-document-button { -// text-align: left; -// } -// } -// } -// -// .search-box { -// .dashicons.dashicons-search { -// margin-left: 10px; -// } -// -// .dokan-admin-search-settings { -// font-size: 10px; -// } -// } -// } -//}
1341-1357: Avoid redundant nested media queriesThere's a nested
@media (max-width: 768px)inside an existing media query with the same condition starting at line 1302. The inner media query is redundant and can be removed to simplify the CSS.Apply this diff to eliminate the redundant media query:
- @media (max-width: 768px) { .dokan-settings-wrap { .nav-tab-wrapper { padding: 10px; .mobile-settings-list { margin-top: 1rem; .setting-section { &.active { border-left: 4px solid #246EFE; } &:hover { background-color: #f9fafb; } } } } } - }src/admin/components/Settings/MobileSettingsList.vue (1)
57-81: Enhance accessibility with ARIA roles and focus managementTo improve accessibility, consider adding ARIA roles such as
role="dialog"to the drawer container. Additionally, manage focus by trapping it within the drawer when it's open and returning focus to the triggering element when it's closed.Example modification:
<div :class="[ 'fixed top-0 left-0 h-full w-[85%] max-w-[320px] bg-white z-[1000000] transform transition-transform duration-300 ease-in-out', isOpen ? 'translate-x-0' : '-translate-x-full' ]" + role="dialog" + aria-modal="true" >Implement focus management in the
mountedandbeforeDestroylifecycle hooks.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
src/admin/components/Settings/MobileSettingsList.vue(1 hunks)src/admin/components/SettingsBanner.vue(1 hunks)src/admin/pages/Settings.vue(9 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/admin/components/SettingsBanner.vue
🔇 Additional comments (3)
src/admin/pages/Settings.vue (2)
115-124: MobileSettingsDrawer component integration looks good
The MobileSettingsDrawer component is properly integrated with the correct props and event handlers, enhancing the mobile user experience.
125-136: Desktop navigation tabs are correctly rendered
The desktop navigation tabs are displayed accurately with appropriate data bindings and event handling.
src/admin/components/Settings/MobileSettingsList.vue (1)
1-54: MobileSettingsDrawer component implementation is well-structured
The MobileSettingsDrawer component is effectively implemented, providing a responsive and user-friendly mobile settings menu. The use of props and event emissions for isOpen, sections, and currentTab ensures flexibility and reusability.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (3)
src/admin/pages/Settings.vue (3)
21-41: Enhance accessibility of the mobile menu toggle button.The button implementation is good, but could be improved for better accessibility.
<button class="d-xs:flex md:hidden p-2.5 bg-white border border-gray-200 rounded-lg shadow-sm hover:bg-gray-50 transition-all duration-200 ease-in-out group focus:outline-none focus:ring-2 focus:ring-blue-500 focus:ring-offset-2" @click="openMobileDrawer" + aria-expanded="showMobileDrawer" + aria-controls="mobile-settings-drawer" > <svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 24 24" width="24" height="24" + aria-hidden="true" class="group-hover:scale-110 transition-transform duration-200" >
1242-1303: Optimize responsive styling implementation.The responsive styling could be improved with better organization and maintainability.
+<style lang="less"> +// Variables +@mobile-breakpoint: 768px; +@drawer-transition-duration: 0.3s; + +// Mixins +.mobile-only { + @media only screen and (max-width: @mobile-breakpoint) { + @content; + } +} @media only screen and (max-width: 768px) { .dokan-settings-wrap { .nav-tab-wrapper { overflow: inherit !important; z-index: 444; width: 100% !important; padding: 1rem !important; } // ... other styles ... .slide-enter-active, .slide-leave-active { - transition: transform 0.3s ease-out; + transition: transform @drawer-transition-duration ease-out; } // Remove duplicate media query - @media (max-width: 768px) { - .dokan-settings-wrap { - // ... duplicate styles ... - } - } } } </style>
Line range hint
1-1304: Consider splitting the component into smaller, more focused components.The Settings component is quite large and handles multiple responsibilities. Consider extracting some functionality into separate components:
- SearchBox component for search functionality
- SettingsHeader component for the header section
- SettingsNavigation component for the navigation tabs
This would improve maintainability and make the code more testable.
Would you like me to provide an example of how to split this component?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
src/admin/components/Settings/MobileSettingsList.vue(1 hunks)src/admin/pages/Settings.vue(9 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/admin/components/Settings/MobileSettingsList.vue
| <transition name="slide"> | ||
| <MobileSettingsDrawer | ||
| :is-open="showMobileDrawer" | ||
| :sections="settingSections" | ||
| :current-tab="currentTab" | ||
| @close="closeMobileDrawer" | ||
| @change-tab="changeTab" | ||
| /> | ||
| </transition> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance mobile drawer usability and accessibility.
The mobile drawer implementation could be improved with better keyboard navigation and click-outside behavior.
<transition name="slide">
<MobileSettingsDrawer
:is-open="showMobileDrawer"
:sections="settingSections"
:current-tab="currentTab"
+ :aria-modal="true"
+ :role="dialog"
+ @keydown.esc="closeMobileDrawer"
@close="closeMobileDrawer"
@change-tab="changeTab"
/>
</transition>
methods: {
openMobileDrawer() {
this.showMobileDrawer = true;
+ document.body.style.overflow = 'hidden'; // Prevent background scroll
this.searchText = '';
this.validateBlankSearch();
+ this.$nextTick(() => {
+ // Focus first interactive element
+ this.$refs.mobileDrawer.$el.querySelector('button,a').focus();
+ });
},
closeMobileDrawer() {
this.showMobileDrawer = false;
+ document.body.style.overflow = ''; // Restore scroll
},
+ handleClickOutside(event) {
+ if (this.showMobileDrawer && !this.$refs.mobileDrawer.$el.contains(event.target)) {
+ this.closeMobileDrawer();
+ }
+ },
},
+mounted() {
+ document.addEventListener('click', this.handleClickOutside);
+},
+beforeDestroy() {
+ document.removeEventListener('click', this.handleClickOutside);
+},Also applies to: 698-710
| <div class="relative flex-1"> | ||
| <!-- Search Box --> | ||
| <div class="search-section relative w-full mb-4"> | ||
| <div class="search-box w-full bg-white rounded-md border border-gray-200 flex items-center"> | ||
| <label for="dokan-admin-search" class="dashicons dashicons-search "></label> | ||
| <input | ||
| type="text" | ||
| autocomplete="off" | ||
| id="dokan-admin-search" | ||
| class="w-full px-3 py-2 border-none focus:outline-none focus:ring-0 bg-transparent " | ||
| :placeholder="__('Search e.g. vendor', 'dokan-lite')" | ||
| v-model="searchText" | ||
| @input="searchInSettings" | ||
| ref="searchInSettings" | ||
| /> | ||
| <span | ||
| v-if="searchText !== ''" | ||
| class="dashicons dashicons-no-alt mr-3 text-gray-400 cursor-pointer hover:text-gray-600" | ||
| @click.prevent="clearSearch" | ||
| ></span> | ||
| </div> | ||
|
|
||
| <!-- Search Suggestions Dropdown --> | ||
| <div | ||
| v-if="searchText && settingSections.length > 0" | ||
| class="d-xs:absolute md:hidden left-0 right-0 top-[calc(100%+4px)] bg-white border border-gray-200 rounded-lg shadow-lg divide-y divide-gray-100 max-h-[280px] overflow-y-auto z-50" | ||
| > | ||
| <div | ||
| v-for="section in settingSections" | ||
| :key="section.id" | ||
| class="hover:bg-gray-50 transition-colors cursor-pointer" | ||
| @click="handleSectionClick(section)" | ||
| > | ||
| <div class="p-3"> | ||
| <div class="flex items-center gap-3"> | ||
| <img | ||
| :src="section.icon_url" | ||
| :alt="section.title" | ||
| class="w-5 h-5" | ||
| /> | ||
| <div class="flex-1 min-w-0"> | ||
| <div class="text-sm font-medium text-gray-900 truncate"> | ||
| {{ section.title }} | ||
| </div> | ||
| <div class="text-xs text-gray-500 mt-0.5 line-clamp-1"> | ||
| {{ section.description }} | ||
| </div> | ||
| </div> | ||
| <svg | ||
| class="w-4 h-4 text-gray-400 flex-shrink-0" | ||
| fill="none" | ||
| stroke="currentColor" | ||
| viewBox="0 0 24 24" | ||
| > | ||
| <path | ||
| stroke-linecap="round" | ||
| stroke-linejoin="round" | ||
| stroke-width="2" | ||
| d="M9 5l7 7-7 7" | ||
| /> | ||
| </svg> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| </div> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve search functionality robustness.
The search implementation could be enhanced in several ways:
- The debounce delay should be configurable
- Add loading state during search
- Handle potential search failures
data () {
return {
// ... other data properties
+ searchDebounceDelay: 300, // Configurable delay
+ isSearching: false,
}
},
methods: {
searchInSettings(input) {
if (!this.validateBlankSearch()) {
return;
}
+ this.isSearching = true;
if (!this.awaitingSearch) {
- setTimeout(() => {
+ setTimeout(async () => {
let searchText = this.$refs.searchInSettings.value;
searchText = searchText.replace(/\s\s+/g,' ').trim().toLowerCase();
this.doSearch('');
- this.doSearch(searchText);
+ try {
+ await this.doSearch(searchText);
+ } catch (error) {
+ console.error('Search failed:', error);
+ // Handle error appropriately
+ } finally {
+ this.isSearching = false;
+ }
this.awaitingSearch = false;
this.showSuggestions = true;
- }, 1000);
+ }, this.searchDebounceDelay);
}
this.awaitingSearch = true;
},Then update the template to show loading state:
<div class="search-box w-full bg-white rounded-md border border-gray-200 flex items-center">
<label for="dokan-admin-search" class="dashicons dashicons-search"></label>
<input
type="text"
autocomplete="off"
id="dokan-admin-search"
class="w-full px-3 py-2 border-none focus:outline-none focus:ring-0 bg-transparent"
:placeholder="__('Search e.g. vendor', 'dokan-lite')"
v-model="searchText"
@input="searchInSettings"
ref="searchInSettings"
+ :disabled="isSearching"
/>
+ <span v-if="isSearching" class="loading-spinner mr-3"></span>
<span
v-if="searchText !== ''"
class="dashicons dashicons-no-alt mr-3 text-gray-400 cursor-pointer hover:text-gray-600"
@click.prevent="clearSearch"
></span>
</div>Committable suggestion skipped: line range outside the PR's diff.
osmansufy
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
src/admin/pages/Settings.vue (2)
Line range hint
204-254: Clean up scroll event listener on component destruction.The scroll event listener is added in the created hook but not removed when the component is destroyed, which could lead to memory leaks.
beforeDestroy() { window.removeEventListener('resize', this.updateDocumentWidth); + window.removeEventListener('scroll', this.handleScroll); }
Line range hint
1107-1306: Optimize styles for better performance.Consider performance optimizations for the styles:
- Combine multiple box-shadow and filter properties
- Use transform: translate3d for hardware acceleration
.search-section { - filter: drop-shadow(0px 0.0869484px 0.260845px rgba(0, 0, 0, 0.1)) drop-shadow(0px 0.869484px 1.73897px rgba(0, 0, 0, 0.2)); + filter: drop-shadow(0px 0.869484px 1.73897px rgba(0, 0, 0, 0.25)); .dashicons.dashicons-no-alt { - transform: translate(0%, -60%); + transform: translate3d(0, -60%, 0); } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/admin/pages/Settings.vue(9 hunks)
🔇 Additional comments (2)
src/admin/pages/Settings.vue (2)
43-108: Previous feedback about search implementation remains applicable.
The search implementation could still benefit from the robustness improvements suggested in the previous review.
113-121: Previous feedback about mobile drawer usability remains applicable.
The mobile drawer implementation could still benefit from the accessibility and usability improvements suggested in the previous review.
| <button | ||
| class="d-xs:flex md:hidden p-2.5 bg-white border border-gray-200 rounded-lg shadow-sm hover:bg-gray-50 transition-all duration-200 ease-in-out group focus:outline-none focus:ring-2 focus:ring-blue-500 focus:ring-offset-2" | ||
| @click="openMobileDrawer" | ||
| > | ||
| <svg | ||
| xmlns="http://www.w3.org/2000/svg" | ||
| viewBox="0 0 24 24" | ||
| width="24" | ||
| height="24" | ||
| class="group-hover:scale-110 transition-transform duration-200" | ||
| > | ||
| <path | ||
| stroke="#246EFE" | ||
| stroke-width="2.5" | ||
| stroke-linecap="round" | ||
| d="M4 6h16M4 12h16M4 18h16" | ||
| class="group-hover:stroke-[#1b4fd1]" | ||
| /> | ||
| </svg> | ||
| <span class="sr-only">{{ __('Toggle Settings Menu', 'dokan-lite') }}</span> | ||
| </button> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance mobile toggle button accessibility.
The mobile toggle button implementation should include additional ARIA attributes for better screen reader support.
<button
class="d-xs:flex md:hidden p-2.5 bg-white border border-gray-200 rounded-lg shadow-sm hover:bg-gray-50 transition-all duration-200 ease-in-out group focus:outline-none focus:ring-2 focus:ring-blue-500 focus:ring-offset-2"
@click="openMobileDrawer"
+ :aria-expanded="showMobileDrawer"
+ aria-controls="mobile-settings-drawer"
+ v-tooltip="__('Toggle Settings Menu', 'dokan-lite')"
>Committable suggestion skipped: line range outside the PR's diff.
osmansufy
added
Type: Enhancement
Needs: Dev Review
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (4)
assets/src/less/global-admin.less (3)
258-262: Consider using CSS Grid for better responsive layout.The flex-direction column layout with gap is good, but CSS Grid could provide more flexibility for complex responsive layouts.
.notice-content { - flex-direction: column; - align-items: start !important; - gap: 1rem; + display: grid; + grid-template-columns: 1fr; + grid-gap: 1rem; + align-items: start; }
Line range hint
1107-1167: Enhance search input accessibility.The search input styling could benefit from improved accessibility features.
input { width: 100%; height: 48px; border: none; display: block; padding: 0 45px 0 0; background: transparent; font-weight: 400; font-family: Roboto, sans-serif; + -webkit-appearance: none; + appearance: none; &:focus { outline: none; box-shadow: none; border-color: transparent; + box-shadow: 0 0 0 2px rgba(36, 110, 254, 0.2); } + + &::placeholder { + color: rgba(60, 60, 67, 0.6); + opacity: 1; + } }
Line range hint
1242-1300: Optimize mobile responsive styles.The mobile styles could be improved for better maintainability and performance.
- Use CSS custom properties for breakpoints
- Consolidate duplicate selectors
- Remove unnecessary
!importantflags+:root { + --mobile-breakpoint: 768px; + --tablet-breakpoint: 1024px; +} -@media only screen and (max-width: 768px) +@media only screen and (max-width: var(--mobile-breakpoint)) .dokan-settings-wrap { .nav-tab-wrapper { - overflow: inherit !important; - z-index: 444; - width: 100% !important; - padding: 1rem !important; + overflow: visible; + z-index: 10; + width: 100%; + padding: 1rem; } // ... rest of the styles }src/admin/pages/Settings.vue (1)
251-254: Consider using Vuex for state management.The component manages multiple pieces of state that could be shared across components. Using Vuex would improve state management and component communication.
Consider moving the following state to Vuex:
screenWidthshowMobileDrawershowSuggestions
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
assets/src/less/global-admin.less(2 hunks)src/admin/pages/Settings.vue(9 hunks)
🔇 Additional comments (2)
assets/src/less/global-admin.less (1)
290-308:
Standardize responsive breakpoints.
The breakpoint range (376px-576px) leaves a 1px gap with the previous media query (max-width: 375px). This could cause layout issues.
-@media screen and (min-width: 376px) and (max-width: 576px)
+@media screen and (min-width: 375px) and (max-width: 576px)Likely invalid or redundant comment.
src/admin/pages/Settings.vue (1)
698-710: Add debounce to search functionality.
The search implementation could benefit from debouncing to prevent excessive API calls.
This issue was previously identified in a past review comment. Please refer to the existing suggestion for implementing debounced search functionality.
assets/src/less/global-admin.less
Outdated
| align-items: start; | ||
| padding: 10px 20px; | ||
| .dokan-message { | ||
| margin: 0 0 0 23px; | ||
| div{ | ||
| max-height: 50px; | ||
| overflow: scroll; | ||
| } | ||
| margin: 0; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve scroll behavior for message content.
Using overflow: scroll forces scrollbars even when unnecessary. Consider using overflow-y: auto for better UX.
.dokan-message {
margin: 0;
div {
- max-height: 50px;
- overflow: scroll;
+ max-height: 50px;
+ overflow-y: auto;
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| align-items: start; | |
| padding: 10px 20px; | |
| .dokan-message { | |
| margin: 0 0 0 23px; | |
| div{ | |
| max-height: 50px; | |
| overflow: scroll; | |
| } | |
| margin: 0; | |
| align-items: start; | |
| padding: 10px 20px; | |
| .dokan-message { | |
| margin: 0; | |
| div { | |
| max-height: 50px; | |
| overflow-y: auto; | |
| } |
| .setting-section { | ||
| &.active { | ||
| border-left: 4px solid #246EFE; | ||
| } | ||
|
|
||
| .nav-description { | ||
| font-size: 8px !important; | ||
| &:hover { | ||
| background-color: #f9fafb; | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance mobile navigation accessibility.
The mobile navigation could benefit from improved keyboard navigation and ARIA attributes.
.setting-section {
+ position: relative;
+ outline: none;
&.active {
border-left: 4px solid #246EFE;
+ background-color: rgba(36, 110, 254, 0.05);
}
&:hover {
background-color: #f9fafb;
}
+ &:focus-visible {
+ box-shadow: 0 0 0 2px #246EFE;
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| .setting-section { | |
| &.active { | |
| border-left: 4px solid #246EFE; | |
| } | |
| .nav-description { | |
| font-size: 8px !important; | |
| &:hover { | |
| background-color: #f9fafb; | |
| } | |
| } | |
| .setting-section { | |
| position: relative; | |
| outline: none; | |
| &.active { | |
| border-left: 4px solid #246EFE; | |
| background-color: rgba(36, 110, 254, 0.05); | |
| } | |
| &:hover { | |
| background-color: #f9fafb; | |
| } | |
| &:focus-visible { | |
| box-shadow: 0 0 0 2px #246EFE; | |
| } | |
| } |
| .slide-enter-active, .slide-leave-active { | ||
| transition: transform 0.3s ease-out; | ||
| } | ||
| } | ||
| } | ||
| .slide-enter, .slide-leave-to { | ||
| transform: translateX(-100%); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve transition performance.
The current transition implementation might cause layout thrashing. Consider using transform-based animations for better performance.
.slide-enter-active, .slide-leave-active {
- transition: transform 0.3s ease-out;
+ transition: transform 0.3s cubic-bezier(0.4, 0, 0.2, 1);
+ will-change: transform;
}
.slide-enter, .slide-leave-to {
transform: translateX(-100%);
+ pointer-events: none;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| .slide-enter-active, .slide-leave-active { | |
| transition: transform 0.3s ease-out; | |
| } | |
| } | |
| } | |
| .slide-enter, .slide-leave-to { | |
| transform: translateX(-100%); | |
| } | |
| .slide-enter-active, .slide-leave-active { | |
| transition: transform 0.3s cubic-bezier(0.4, 0, 0.2, 1); | |
| will-change: transform; | |
| } | |
| .slide-enter, .slide-leave-to { | |
| transform: translateX(-100%); | |
| pointer-events: none; | |
| } |
| .notice-content{ | ||
| flex-direction: column; | ||
| align-items: start !important; | ||
| gap: 1rem; | ||
| } |
There was a problem hiding this comment.
This section is also used for 576px. Could remove this replication?
| @@ -276,21 +287,33 @@ | |||
|
|
|||
| @media screen and (min-width: 376px) and (max-width: 576px) { | |||
There was a problem hiding this comment.
Should we care about (min-width: 376px) in the above code section? Pls see the recommended screen sizes
There was a problem hiding this comment.
Bhai,
@media screen was impliment prev PR .
I think we can skip this refactor as it is working
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/admin/components/AdminNotice.vue (1)
16-21: Consider improving accessibility for action buttons.While the structural change is good, consider enhancing accessibility:
-<div> +<div class="dokan-notice-actions" role="group" aria-label="Notice Actions">This change would:
- Add semantic meaning to the button group
- Improve screen reader navigation
- Follow ARIA best practices
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
assets/src/less/global-admin.less(2 hunks)src/admin/components/AdminNotice.vue(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- assets/src/less/global-admin.less
🔇 Additional comments (1)
src/admin/components/AdminNotice.vue (1)
16-21: Verify CSS changes for the new action buttons wrapper.
The addition of a wrapper div around action buttons might require corresponding CSS changes to maintain proper styling, especially for mobile responsiveness.
✅ Verification successful
The wrapper div is consistent with existing CSS styles
The CSS inspection shows that the admin notice component already has styles handling div elements within .dokan-message, including specific mobile-friendly rules for content overflow:
.dokan-message {
div {
max-height: 50px;
overflow: auto;
}
margin: 0;
}The addition of the wrapper div around action buttons aligns with the existing CSS structure and mobile responsiveness implementation.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for related CSS changes in global-admin.less and other style files
# Search for CSS selectors targeting notice action buttons
rg -t less "dokan-admin-notice.*dokan-btn"
# Search for any new CSS rules added for the action buttons wrapper
rg -t less "dokan-message.*div"
Length of output: 78
Script:
#!/bin/bash
# Let's try different search patterns to find CSS related to admin notices and buttons
# Search for CSS rules related to admin notices
rg -t less "\.dokan-admin-notice"
# Search for button styles within admin-related CSS
rg -t less "\.dokan-btn"
# Search for any CSS files that might contain relevant styles
fd -e less -e css
Length of output: 5960
Script:
#!/bin/bash
# Let's check the specific CSS rules for admin notices and their buttons in global-admin.less
# Get the content of admin notice styles with context
rg -A 10 "\.dokan-admin-notice \{" assets/src/less/global-admin.less
# Check for any mobile/responsive styles for admin notices
rg -A 10 "@media.*dokan-admin-notice" assets/src/less/global-admin.less
rg -A 10 "@media.*dokan-admin-notice" assets/src/less/responsive.less
Length of output: 1521
All Submissions:
Changes proposed in this Pull Request:
Related Pull Request(s)
Closes
How to test the changes in this Pull Request:
Changelog entry
Detailed Description of the pull request. What was previous behaviour
and what will be changed in this PR.
Settings page responsive for mobile version with drower & search functionality
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
Summary by CodeRabbit
New Features
Bug Fixes
Documentation