Add Inertia to the Table #1
Conversation
WalkthroughCette mise à jour remanie en profondeur la gestion des tableaux de données côté client et serveur, introduit l’internationalisation (i18n) avec prise en charge de l’anglais et du français, et remplace la gestion locale de l’état par une synchronisation serveur via Inertia.js. Des types TypeScript structurent désormais la configuration des colonnes, actions et filtres. Plusieurs fichiers de données statiques sont supprimés. La visibilité des colonnes est persistée en session et synchronisée avec le serveur. Le rendu côté serveur est simplifié, et les tests sont réorganisés. Changes
Sequence Diagram(s)sequenceDiagram
participant Utilisateur
participant DataTable.vue
participant Inertia.js
participant Serveur (Laravel)
participant i18n
Utilisateur->>DataTable.vue: Interagit (trier, filtrer, sélectionner)
DataTable.vue->>Inertia.js: Envoie requête (état: tri, filtres, sélection)
Inertia.js->>Serveur (Laravel): Requête AJAX (état du tableau)
Serveur (Laravel)->>Serveur (Laravel): Applique logique (visibilité, tri, actions)
Serveur (Laravel)-->>Inertia.js: Renvoie nouvelles données/état
Inertia.js-->>DataTable.vue: Met à jour la vue
DataTable.vue->>i18n: Traduit les labels et messages
i18n-->>DataTable.vue: Retourne les chaînes localisées
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (2)
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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: 14
🔭 Outside diff range comments (1)
resources/js/components/type.ts (1)
119-124: Éviter l'utilisation deanypour la propriétéiconsLa propriété
iconsest typée commeany, ce qui devrait être évité en TypeScript.export interface DatatableProps { route?: string; name: string; - icons?: any; + icons?: Record<string, any> | typeof LucideIcons; }
🧹 Nitpick comments (9)
resources/js/components/ui/dropdown-menu/DropdownMenuPortal.vue (1)
5-10: Améliorer le typage des props pour une meilleure expérience développeur.Les props
containeretasavectype: nullne fournissent pas un typage optimal. Considérez des types plus spécifiques commeHTMLElementpourcontaineretStringpouras.const props = defineProps({ forceMount: { type: Boolean, required: false }, - container: { type: null, required: false }, + container: { type: HTMLElement, required: false }, asChild: { type: Boolean, required: false }, - as: { type: null, required: false }, + as: { type: String, required: false }, });resources/js/components/DataTableColumnHeader.vue (1)
26-33: Supprimez l'appel redondant dansonMounted.La fonction
updateSortDirectionest appelée à la fois dansonMountedet dans lewatchavecimmediate: true. Cela provoque une double exécution lors du montage initial du composant.Supprimez le hook
onMountedcar le watcher avecimmediate: truegère déjà l'initialisation :-// Initialize sortDirection based on props -onMounted(() => { - updateSortDirection() -}) - // Update sortDirection when props change watch([() => props.currentSort, () => props.currentDirection], () => { updateSortDirection() -}) +}, { immediate: true })resources/js/components/DataTableFacetedFilter.vue (1)
71-73: Ajoutez une gestion d'erreur pour la résolution des icônes.La fonction retourne silencieusement
nullsi l'icône n'est pas trouvée, ce qui peut rendre le débogage difficile.const getIconComponent = (iconName) => { - return typeof iconName === 'string' ? LucideIcons[iconName] || null : iconName + if (typeof iconName === 'string') { + const icon = LucideIcons[iconName] + if (!icon && import.meta.env.DEV) { + console.warn(`Icon "${iconName}" not found in LucideIcons`) + } + return icon || null + } + return iconName }resources/js/components/DataTableViewOptions.vue (1)
87-87: Utilisez le router Inertia au lieu dewindow.location.L'utilisation directe de
window.location.pathnamerend le code difficile à tester.-router.post(window.location.pathname, params, { +router.post(router.page.url, params, {Ou utilisez
usePage().urlpour obtenir l'URL actuelle via Inertia.resources/js/components/type.ts (2)
46-53: Clarifier la relation entreoptionsetfilterOptionsL'interface a deux propriétés pour les options :
options: Record<string, string>etfilterOptions?: FilterOption[]. Leur relation et usage respectif ne sont pas clairs.Considérez d'ajouter des commentaires JSDoc pour clarifier l'usage :
export interface FilterDefinition { name: string; label: string; + /** Legacy options format for backward compatibility */ options: Record<string, string>; icons?: Record<string, string>; multiple: boolean; + /** Enhanced options with icons and counts */ filterOptions?: FilterOption[]; }
69-73: Supprimer le code commentéLe code commenté devrait être supprimé pour maintenir la propreté du code.
-// export interface FormattedData { -// [key: string]: string | number | boolean | null; -// _id?: number | string; -// id?: number | string; -// }src/InertiaDatatable.php (2)
231-246: Supprimer le code commentéUn grand bloc de code commenté devrait être supprimé. Si nécessaire, utilisez le contrôle de version pour retrouver l'ancienne implémentation.
-//public function render(string $component): JsonResponse|Response|BinaryFileResponse -//{ -// if (!isset($this->table)) { -// throw new Error('No table set for datatable'); -// } -// -// $props = $this->getProps(); -// -// $request = $this->getRequest(); -// // Handle export if requested -// if ($request->has('export')) { -// return $this->handleExport(); -// } -// -// return Inertia::render($component, $props); -//}
309-314: Bonne implémentation de la fusion des états de visibilitéLa fusion des états de visibilité permet des mises à jour incrémentielles, ce qui est une bonne approche.
Considérez d'utiliser
array_replacepour une syntaxe plus claire :-$value = array_merge($existingValue, $value); +$value = array_replace($existingValue, $value);
array_replaceest plus approprié ici car il remplace les valeurs avec les mêmes clés plutôt que de les ajouter.resources/js/components/DataTableToolbar.vue (1)
144-145: Traduire les commentaires en anglaisLes commentaires devraient être en anglais pour maintenir la cohérence du code.
-// Ajouter les IDs sélectionnés si nécessaire +// Add selected IDs if necessary
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (30)
package.json(1 hunks)resources/js/DataTable.vue(1 hunks)resources/js/components/DataTable.vue(2 hunks)resources/js/components/DataTableColumnHeader.vue(1 hunks)resources/js/components/DataTableFacetedFilter.vue(4 hunks)resources/js/components/DataTablePagination.vue(1 hunks)resources/js/components/DataTableToolbar.vue(1 hunks)resources/js/components/DataTableViewOptions.vue(2 hunks)resources/js/components/columns.ts(0 hunks)resources/js/components/type.ts(1 hunks)resources/js/components/ui/dialog/DialogContent.vue(1 hunks)resources/js/components/ui/dialog/DialogScrollContent.vue(0 hunks)resources/js/components/ui/dropdown-menu/DropdownMenu.vue(1 hunks)resources/js/components/ui/dropdown-menu/DropdownMenuContent.vue(1 hunks)resources/js/components/ui/dropdown-menu/DropdownMenuPortal.vue(1 hunks)resources/js/data/data.ts(0 hunks)resources/js/data/schema.ts(0 hunks)resources/js/data/tasks.json(0 hunks)resources/js/i18n/index.js(1 hunks)resources/js/i18n/locales/en.json(1 hunks)resources/js/i18n/locales/fr.json(1 hunks)resources/js/i18n/useTranslation.js(1 hunks)resources/js/index.js(1 hunks)resources/js/main.js(1 hunks)src/Columns/CheckboxColumn.php(2 hunks)src/Columns/Column.php(6 hunks)src/InertiaDatatable.php(11 hunks)src/Services/ExportService.php(1 hunks)tests/Unit/InertiaDatatableTest.php(1 hunks)vite.config.js(1 hunks)
💤 Files with no reviewable changes (5)
- resources/js/components/columns.ts
- resources/js/components/ui/dialog/DialogScrollContent.vue
- resources/js/data/schema.ts
- resources/js/data/tasks.json
- resources/js/data/data.ts
🧰 Additional context used
🧬 Code Graph Analysis (8)
src/Services/ExportService.php (1)
src/Table.php (1)
exportType(61-68)
src/Columns/CheckboxColumn.php (7)
src/Columns/Column.php (2)
label(187-192)getHidden(254-257)src/Actions/TableAction.php (1)
label(32-37)src/Actions/TableActionGroup.php (1)
label(30-35)src/Columns/ColumnAction.php (1)
label(36-41)src/Columns/ColumnActionGroup.php (1)
label(26-31)src/Filters/Filter.php (1)
label(34-39)src/Filters/FilterOption.php (1)
label(33-38)
resources/js/main.js (2)
resources/js/i18n/index.js (1)
i18n(5-10)resources/js/i18n/useTranslation.js (2)
useTranslation(7-40)t(14-37)
resources/js/i18n/useTranslation.js (2)
resources/js/main.js (1)
useTranslation(15-15)resources/js/i18n/index.js (1)
i18n(5-10)
tests/Unit/InertiaDatatableTest.php (6)
tests/TestModels/TestModelDataTable.php (1)
TestModelDataTable(7-12)src/InertiaDatatable.php (5)
getFromSession(259-262)getSessionKey(40-45)table(49-54)make(189-208)getColumns(363-396)src/EloquentTable.php (2)
EloquentTable(9-30)make(21-24)src/Columns/Column.php (2)
make(28-43)Column(10-301)tests/TestModels/TestModel.php (1)
TestModel(9-24)src/Table.php (2)
columns(17-21)getColumns(35-38)
src/Columns/Column.php (6)
src/Columns/CheckboxColumn.php (1)
make(17-35)src/Columns/ColumnAction.php (3)
make(23-29)getName(31-34)getLabel(43-46)src/Columns/ColumnActionGroup.php (2)
make(17-24)getLabel(33-36)src/Filters/Filter.php (3)
make(20-27)getName(29-32)getLabel(125-128)src/Filters/FilterOption.php (3)
make(19-26)getLabel(40-43)getIconPosition(55-58)src/Traits/HasIcon.php (2)
hasIcon(70-73)getIconPosition(60-63)
resources/js/components/type.ts (1)
src/Columns/Column.php (1)
Column(10-301)
src/InertiaDatatable.php (4)
src/EloquentTable.php (1)
getQuery(26-29)tests/Unit/InertiaDatatableTest.php (5)
get(834-843)get(861-870)get(887-896)get(913-922)getFromSession(1661-1671)src/Table.php (2)
getColumns(35-38)columns(17-21)src/Columns/Column.php (3)
getName(45-48)hidden(247-252)toArray(287-300)
🔇 Additional comments (25)
resources/js/components/ui/dialog/DialogContent.vue (2)
13-19: Aucune modification nécessaire : la gestion du focus est toujours assurée par Reka UI.
DansDialogContent.vue, on retrouve bien les émissions d’événements liées au focus (focusOutside,openAutoFocus,closeAutoFocus), héritées de Headless UI via Reka UI, assurant le verrouillage du focus et l’autofocus automatique sans besoin de la proptrapFocus.
43-43: Animations cohérentes : simplification validéeAprès comparaison des classes d’animation :
- DialogScrollContent.vue applique uniquement les effets de fondu (animate-in/animate-out + fade-in/fade-out).
- DialogContent.vue conserve à la fois fondu et zoom (animate-in/animate-out + fade-in/fade-out + zoom-in/zoom-out) pour le contenu centré.
Aucune divergence détectée : la simplification est cohérente entre les composants de dialogue.
vite.config.js (1)
30-30: Modification cosmétique approuvée.L'ajout d'une nouvelle ligne à la fin du fichier respecte les bonnes pratiques de formatage et n'a aucun impact sur la fonctionnalité.
package.json (2)
29-29: Mise à jour de dépendance approuvée.La mise à jour de reka-ui vers la version 2.3.2 est une mise à jour mineure appropriée qui devrait inclure des corrections de bugs ou des améliorations de sécurité.
33-33: Ajout de vue-i18n approuvé.L'ajout de vue-i18n v11.1.9 est cohérent avec les objectifs du PR d'introduire l'internationalisation. Cette version est récente et stable.
resources/js/components/ui/dropdown-menu/DropdownMenuContent.vue (1)
38-39: Ajout d'événements approuvé.L'ajout des événements "openAutoFocus" et "select" standardise l'interface d'événements à travers les composants dropdown menu et améliore la gestion du focus et de la sélection.
resources/js/components/ui/dropdown-menu/DropdownMenu.vue (1)
10-10: Standardisation des événements approuvée.L'ajout de l'événement "openAutoFocus" aligne ce composant avec les autres composants dropdown menu et améliore la gestion cohérente du focus automatique.
resources/js/i18n/index.js (1)
1-12: Configuration i18n bien structurée !La configuration de l'internationalisation est correctement implémentée avec vue-i18n. Le mode legacy est approprié pour Vue 3, et le choix du français comme langue par défaut avec l'anglais comme fallback est cohérent avec les objectifs du projet.
src/Columns/CheckboxColumn.php (2)
21-21: Initialisation explicite du label appropriée.L'initialisation explicite du label à une chaîne vide est cohérente avec l'usage des colonnes checkbox qui n'ont généralement pas besoin d'étiquette visible.
112-112: Intégration correcte du système de visibilité.L'ajout de la propriété 'hidden' dans le tableau retourné s'aligne parfaitement avec le nouveau système de gestion de visibilité des colonnes introduit dans cette PR.
resources/js/i18n/locales/en.json (1)
1-31: Fichier de localisation anglaise complet et bien structuré.Le fichier de localisation couvre tous les éléments nécessaires pour l'interface utilisateur des tableaux de données avec des traductions appropriées et des placeholders correctement définis pour les valeurs dynamiques.
resources/js/main.js (1)
7-22: Configuration d'application Vue 3 avec Inertia.js bien implémentée.La configuration intègre correctement Inertia.js et l'internationalisation. L'utilisation du système provide/inject de Vue pour fournir la fonction de traduction globalement est une excellente pratique qui facilite l'accès aux traductions dans tous les composants.
resources/js/i18n/locales/fr.json (1)
1-31: Fichier de localisation française de qualité.Les traductions françaises sont naturelles et appropriées pour le contexte d'une interface de tableau de données. La cohérence avec le fichier de localisation anglaise est parfaitement maintenue, et les placeholders pour les valeurs dynamiques sont correctement préservés.
resources/js/index.js (1)
2-9: Excellente organisation modulaire.L'approche de centralisation des exports suit les bonnes pratiques et facilite l'utilisation des composants et utilitaires dans l'ensemble de l'application.
resources/js/DataTable.vue (1)
3-21: Refactoring réussi vers une architecture pilotée par le serveur.L'intégration d'Inertia.js et la gestion des traductions via l'injection de dépendances sont bien implémentées. La prop
namerequise permet une identification claire des instances de tableau.src/Columns/Column.php (3)
26-26: Propriétéhiddenajoutée de manière cohérente.L'ajout de la propriété
$hiddensuit les conventions établies dans la classe et s'intègre bien avec les autres propriétés booléennes.
247-257: Méthodes getter/setter bien implémentées.Les méthodes
hidden()etgetHidden()suivent le même pattern que les autres propriétés de la classe, offrant une API cohérente.
298-298: Intégration correcte dans la sérialisation.L'ajout de
'hidden'dans la méthodetoArray()assure que l'état de visibilité est correctement transmis au frontend.tests/Unit/InertiaDatatableTest.php (1)
1657-1710: Test bien structuré pour la visibilité des colonnes depuis la session.L'implémentation du test est correcte et suit les bonnes pratiques. Le test vérifie efficacement que les paramètres de visibilité stockés en session sont appliqués aux colonnes.
resources/js/components/DataTableColumnHeader.vue (1)
69-69: Vérifiez le comportement d'affichage des colonnes.La condition actuelle n'affiche rien si la colonne n'a pas de label ou n'est ni triable ni masquable. Cela pourrait créer des incohérences dans l'interface utilisateur.
Assurez-vous que c'est le comportement souhaité. Certaines colonnes pourraient disparaître complètement de l'en-tête du tableau.
src/InertiaDatatable.php (1)
149-149: Bonne pratique : utilisation de clone()L'ajout de
clone()évite les effets de bord sur le query builder original.resources/js/components/DataTableToolbar.vue (1)
135-154: Implémentation correcte de la fonctionnalité d'exportLa logique d'export est bien implémentée avec une gestion appropriée des paramètres et des options.
resources/js/components/DataTable.vue (3)
106-117: Vérifier la réactivité lors de la modification des propriétés de colonnesLa modification directe de
column.hiddenpourrait ne pas déclencher correctement la réactivité Vue.Si les mises à jour de l'interface ne se reflètent pas correctement, considérez d'utiliser une approche plus réactive ou de forcer la mise à jour du composant.
90-99: Excellente gestion de la sélection avec prise en compte des lignes désactivéesLa logique de sélection gère correctement les lignes désactivées et l'état indéterminé.
241-311: Système de confirmation robuste avec gestion des fallbacksLe système de dialogue de confirmation est bien implémenté avec une gestion appropriée des états et des fallbacks.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
resources/js/components/DataTableToolbar.vue (1)
46-60: Memory leak: Timeout cleanup still missingThe searchTimeout is not cleaned up when the component is destroyed, which can cause memory leaks. This issue was previously identified but hasn't been addressed.
Add the cleanup in an onUnmounted hook:
-import {computed, reactive, watch, onMounted, inject, provide} from 'vue' +import {computed, reactive, watch, onMounted, onUnmounted, inject, provide} from 'vue'And add after line 161:
+onUnmounted(() => { + if (searchTimeout) { + clearTimeout(searchTimeout); + } +});
🧹 Nitpick comments (3)
resources/js/components/DataTableToolbar.vue (3)
135-154: Improve export implementation with better error handlingThe export functionality uses
window.location.hrefwhich doesn't provide feedback on failures and could be improved.Consider using a more robust approach with proper error handling:
-// Déclencher le téléchargement - window.location.href = url; +// Déclencher le téléchargement avec gestion d'erreur + try { + const link = document.createElement('a'); + link.href = url; + link.download = ''; + document.body.appendChild(link); + link.click(); + document.body.removeChild(link); + } catch (error) { + console.error('Export failed:', error); + // Fallback to window.location.href + window.location.href = url; + }
56-58: Deep watching performance considerationDeep watching the filters object could have performance implications with large filter sets.
Consider using a more targeted approach or debouncing for better performance:
-watch(() => form.filters, (newValue) => { - handleFilters(newValue); -}, {deep: true}); +watch(() => form.filters, (newValue) => { + // Debounce filter changes too + if (filterTimeout) clearTimeout(filterTimeout); + filterTimeout = setTimeout(() => { + handleFilters(newValue); + }, 100); +}, {deep: true});
28-31: Improve form initialization type safetyThe reactive form could benefit from better TypeScript typing for the filters property.
-const form = reactive({ - search: null, - filters: {} as Record<string, string[]> -}) +const form = reactive({ + search: null as string | null, + filters: {} as Record<string, string[]> +})
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
resources/js/components/DataTableToolbar.vue(1 hunks)src/Services/ExportService.php(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/Services/ExportService.php
🔇 Additional comments (3)
resources/js/components/DataTableToolbar.vue (3)
168-170: LGTM: Good internationalization integrationThe search input properly uses the translation function and reactive binding.
172-181: LGTM: Well-structured dynamic filter renderingThe dynamic filter generation properly handles both filterOptions and legacy options formats with good reactivity.
208-217: Icon rendering is securely restricted to known LucideIconsgetIconComponent only looks up
LucideIcons[iconName]and falls back tonull(or returns a passed-in component), so arbitrary strings won’t render unknown components.• resources/js/components/DataTableToolbar.vue @ L122
• resources/js/components/DataTableFacetedFilter.vue @ L71
• resources/js/components/DataTable.vue @ L172
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
resources/js/components/DataTableToolbar.vue (1)
207-207: Fix missing selectedRows count display.The template has an empty interpolation
{{}}before the translation. This should display the count of selected rows.-<span v-if="hasSelectedRows" class="text-sm font-medium">{{}} {{ t('selected') }}</span> +<span v-if="hasSelectedRows" class="text-sm font-medium">{{ props.selectedRows.length }} {{ t('selected') }}</span>
🧹 Nitpick comments (1)
resources/js/components/DataTable.vue (1)
241-273: Consider simplifying the complex watch logic.The nested watchers for confirmation dialogs and data changes are quite complex. While functional, consider extracting some of this logic into separate composables for better maintainability.
// Consider creating a composable like useConfirmationDialog const useConfirmationDialog = () => { const showDialog = ref(false) const pendingAction = ref(null) const pendingRowId = ref(null) // Extract confirmation logic here return { showDialog, pendingAction, pendingRowId, handleConfirm, handleCancel } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
resources/js/components/DataTable.vue(2 hunks)resources/js/components/DataTableToolbar.vue(1 hunks)resources/js/components/type.ts(1 hunks)resources/js/i18n/useTranslation.js(1 hunks)src/InertiaDatatable.php(10 hunks)src/Services/ExportService.php(0 hunks)tests/Unit/InertiaDatatableExportTest.php(1 hunks)tests/Unit/InertiaDatatableHandleExportTest.php(1 hunks)tests/Unit/InertiaDatatableTest.php(1 hunks)
💤 Files with no reviewable changes (1)
- src/Services/ExportService.php
🚧 Files skipped from review as they are similar to previous changes (3)
- resources/js/i18n/useTranslation.js
- resources/js/components/type.ts
- src/InertiaDatatable.php
🔇 Additional comments (9)
tests/Unit/InertiaDatatableHandleExportTest.php (1)
112-143: LGTM! Well-structured test for export permission validation.The new test method properly verifies that non-exportable tables throw a 403 exception when export is attempted. The test follows good practices by creating a real datatable instance and properly setting up the test scenario.
tests/Unit/InertiaDatatableExportTest.php (1)
33-36: Export functionality tests are adequately coveredExport-related tests have been moved to dedicated suites, so coverage is intact. Identified test files:
- tests/Unit/InertiaDatatableHandleExportTest.php
- tests/Unit/InertiaDatatableTest.php
- tests/Unit/TableTest.php
- tests/Unit/ExportServiceTest.php
- tests/Unit/ColumnExportTest.php
No further action required.
tests/Unit/InertiaDatatableTest.php (1)
1585-1638: LGTM! Good test coverage for session-based column visibility.The new test method effectively verifies that column visibility settings from session data are correctly applied. The use of an anonymous subclass to override
getFromSessionis a clean testing approach that allows for controlled testing of the visibility logic.resources/js/components/DataTableToolbar.vue (3)
115-119: Good implementation of cleanup in onUnmounted.The timeout cleanup in the
onUnmountedlifecycle hook properly prevents memory leaks by clearing the search timeout when the component is destroyed.
127-130: Good TypeScript typing for icon component function.The
getIconComponentfunction now has proper TypeScript type annotations, improving type safety and preventing potential runtime errors.
132-159: Well-implemented export functionality.The export handling logic is comprehensive, supporting different export formats (CSV/Excel), column selection (visible/all), and row selection (all/selected). The URL construction and parameter handling is clean and follows good practices.
resources/js/components/DataTable.vue (3)
58-87: Well-implemented row selection logic.The row selection functionality is properly implemented with reactive state management. The use of
Setfor tracking selected rows is efficient, and the toggle functions properly handle edge cases like disabled checkboxes.
105-117: Good reactive column visibility management.The watcher for
visibleColumnsproperly updates column hidden properties, ensuring that column visibility changes are reflected immediately in the UI while maintaining server synchronization.
387-441: Robust table rendering with proper null checks.The table rendering logic properly handles different column types (checkbox, action, icon) and includes appropriate null checks to prevent runtime errors. The use of v-html for dynamic content is acceptable here since the content comes from the server.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
resources/js/components/DataTableFacetedFilter.vue (1)
4-4: Bundle size impact from wildcard import persists.The wildcard import of all Lucide icons still impacts bundle size significantly, as previously noted.
🧹 Nitpick comments (1)
resources/js/components/DataTableFacetedFilter.vue (1)
35-67: Optimize Set-based state management for better reactivity.The Set-based approach is efficient, but there are opportunities for improvement:
- The computed property may not trigger reactivity reliably with Set operations
- Consider using
shallowReffor better performance with SetsApply this diff to improve reactivity and performance:
-const selectedValuesSet = ref(new Set<string>()) +const selectedValuesSet = shallowRef(new Set<string>()) -// Computed property to get the selected values as an array -const selectedValues = computed(() => selectedValuesSet.value) +// Computed property to get the selected values as an array +const selectedValues = computed(() => { + // Access .value to ensure reactivity triggers + return selectedValuesSet.value +})This ensures proper reactivity while maintaining the Set's performance benefits.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
resources/js/components/DataTableFacetedFilter.vue(4 hunks)src/InertiaDatatable.php(10 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/InertiaDatatable.php
🔇 Additional comments (4)
resources/js/components/DataTableFacetedFilter.vue (4)
25-27: Interface extensions look good.The addition of optional
countandselectedproperties appropriately extends the component's functionality while maintaining backward compatibility.
31-33: Proper event emission pattern implemented.The emit definition correctly follows Vue 3 patterns for typed component events.
74-74: Translation implementation follows best practices.The
useTranslationcomposable is properly integrated and used consistently throughout the component.
119-156: Template integration with new state management is well-executed.The template changes properly integrate with the new internal state management, internationalization, and dynamic icon resolution. The conditional rendering and event handling are implemented correctly.
| // Function to get the icon component by name | ||
| const getIconComponent = (iconName) => { | ||
| return typeof iconName === 'string' ? LucideIcons[iconName] || null : iconName | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve type safety and performance of icon resolution.
The getIconComponent function lacks proper typing and may impact performance when called during renders.
Apply this diff to improve type safety and performance:
-// Function to get the icon component by name
-const getIconComponent = (iconName) => {
- return typeof iconName === 'string' ? LucideIcons[iconName] || null : iconName
-}
+// Function to get the icon component by name
+const getIconComponent = (iconName: string | Component | undefined): Component | null => {
+ if (!iconName) return null
+ return typeof iconName === 'string' ? LucideIcons[iconName] || null : iconName
+}Consider memoizing this function or pre-computing icon mappings if performance becomes an issue.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In resources/js/components/DataTableFacetedFilter.vue around lines 69 to 72, the
getIconComponent function lacks explicit typing and may cause performance issues
if called repeatedly during renders. To fix this, add proper TypeScript types to
the function parameters and return type to improve type safety. Additionally,
optimize performance by memoizing the function result or pre-computing a mapping
of icon names to components outside the function to avoid repeated lookups
during rendering.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
resources/js/components/DataTableFacetedFilter.vue (1)
74-103: Fix type safety issues in icon component resolution.The
getIconComponentfunction lacks proper TypeScript types and may have runtime issues with dynamic imports.Apply this diff to improve type safety:
-// Function to get the icon component by name -const getIconComponent = (iconName) => { +// Function to get the icon component by name +const getIconComponent = (iconName: string | Component | undefined): Component | null => { + if (!iconName) return null // If iconName is not a string, it's already a component if (typeof iconName !== 'string') { return iconName }Additionally, consider memoizing the dynamic import results to avoid repeated async operations during renders.
🧹 Nitpick comments (1)
resources/js/components/DataTableFacetedFilter.vue (1)
92-94: Consider error handling for dynamic icon imports.The dynamic import may fail if the icon name doesn't correspond to an existing Lucide icon file. Consider adding better error handling or fallback mechanisms.
// For other icons, try to load them dynamically const asyncIcon = defineAsyncComponent(() => - import(`lucide-vue-next/dist/esm/icons/${iconName.toLowerCase()}`).then(module => module.default || module) + import(`lucide-vue-next/dist/esm/icons/${iconName.toLowerCase()}`).then(module => module.default || module) + .catch(error => { + console.warn(`Icon ${iconName} not found, using fallback`) + return null + }) )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
resources/js/components/DataTableFacetedFilter.vue(4 hunks)resources/js/components/ui/command/CommandDialog.vue(1 hunks)resources/js/components/ui/dialog/Dialog.vue(1 hunks)resources/js/components/ui/dropdown-menu/DropdownMenu.vue(1 hunks)resources/js/components/ui/input/Input.vue(2 hunks)resources/js/components/ui/popover/Popover.vue(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- resources/js/components/ui/command/CommandDialog.vue
- resources/js/components/ui/dialog/Dialog.vue
- resources/js/components/ui/popover/Popover.vue
🚧 Files skipped from review as they are similar to previous changes (1)
- resources/js/components/ui/dropdown-menu/DropdownMenu.vue
🔇 Additional comments (6)
resources/js/components/ui/input/Input.vue (1)
4-4: LGTM! Clean implementation of programmatic focus control.The addition of the
focusmethod viadefineExposeis well-implemented and follows Vue 3 composition API patterns correctly. The template ref is properly assigned and the method safely handles potential null values.Also applies to: 19-19, 21-26, 31-31
resources/js/components/DataTableFacetedFilter.vue (5)
39-48: LGTM! Proper reactive state initialization and synchronization.The reactive state management using
Setfor selected values is well-implemented. The watcher correctly synchronizes with theselectedprop and handles both initialization and updates properly.
54-71: LGTM! Clean selection management with proper event emission.The
updateSelectedandclearSelectedmethods are well-implemented with proper state management and event emission. The use ofSetoperations is efficient and the state synchronization is correct.
105-105: LGTM! Proper i18n integration.The integration of the
useTranslationcomposable is clean and follows the established pattern for internationalization in the project.
150-150: LGTM! Consistent internationalization implementation.The translation keys are properly used for user-facing text, maintaining consistency with the broader i18n implementation across the project.
Also applies to: 187-187
156-159: LGTM! Clean event handling with proper state management.The selection toggle logic is well-implemented with proper state updates and event emission. The use of functional updates ensures reactivity is maintained.
Summary by CodeRabbit
Nouvelles fonctionnalités
Corrections de bugs
Refactorisation
falseaux propsmodaldans plusieurs composants UI.Tests
Chores
modal.