Conversation
Summary of ChangesHello @Bilaljanir, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the user experience and functionality of the form creation and editing pages. It introduces a more flexible and robust dynamic schedule calculator, improves search capabilities, and refines the overall form layout and user feedback mechanisms. The changes aim to make data entry and management more intuitive and efficient. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request significantly refactors and improves the form creation and editing views, particularly with the introduction of a new dynamic planning calculator, which is a great enhancement. The code is now cleaner and more maintainable. However, I've identified a few regressions in the edit form (edit.blade.php) that need attention: the UI for managing custom fields has been removed, a required attribute is missing from a key field, and a client-side validation on form submission has been removed. I've also included a minor UX suggestion for the create form.
I am having trouble creating individual review comments. Click here to see my feedback.
resources/views/forms/edit.blade.php (48)
The functionality to manage custom fields (edit, add, delete) has been removed from the UI in this file. The x-data="cdcFormBuilder()" directive was removed from the <form> tag, and all associated HTML and JavaScript for handling custom fields are gone. However, the backend FormController@update method still expects and processes data for existing, new, and deleted custom fields. This is a critical feature regression that prevents users from managing custom fields on existing forms. Please restore the UI for custom field management.
resources/views/forms/edit.blade.php (386-392)
The descriptif_projet field is marked as required in the FormController@update validation rules, but the required attribute is missing from the <x-markdown-editor> component here. This will allow form submission with an empty project description, which will then fail server-side validation. Please add the required attribute back.
<x-markdown-editor
name="descriptif_projet"
:value="$val('descriptif_projet')"
label="Description complète du projet"
placeholder="Le projet consiste à réaliser une application..."
help="Utilisez Markdown pour structurer et formater votre texte"
required
/>resources/views/forms/edit.blade.php (742-793)
The previous version of this file included a JavaScript listener on form submission to prevent submitting the form if the planning distribution was invalid (e.g., total hours exceeded or percentage was not 100%). This client-side validation has been removed. While the new UI displays a warning, it doesn't block the submission, leading to a poorer user experience as the user will only see the validation error after a full page reload. It is recommended to re-implement the client-side submission block.
resources/views/forms/create.blade.php (585-587)
The confirmation dialog for removing a custom field has been removed. This could lead to users accidentally deleting fields. It's recommended to re-introduce the confirm() check to improve user experience.
removeField(index) {
if (confirm('Êtes-vous sûr de vouloir supprimer ce champ ?')) {
this.fields.splice(index, 1);
}
}
|
|
||
| if ($request->filled('search')) { | ||
| $search = $request->search; | ||
| $search = strtolower($request->search); |
There was a problem hiding this comment.
Need to use https://www.php.net/manual/en/function.mb-strtolower.php
In php, we should always use mb_ prefixed function when working with strings. These functions support multi bytes charachters encoding like utf-8
In laravel you can use Str::lower()
| $search = strtolower($request->search); | ||
| $query->where(function($q) use ($search) { | ||
| $q->where('name', 'ILIKE', '%' . $search . '%'); | ||
| $q->whereRaw('LOWER(name) LIKE ?', ["%{$search}%"]); |
There was a problem hiding this comment.
Here you put again in lowercase with SQL, why putting two times in lowercase.
This is wasting cpu cycles
| @@ -562,89 +471,119 @@ class="px-6 py-3 bg-green-600 text-white rounded-md hover:bg-green-700 transitio | |||
| </div> | |||
|
|
|||
| <script> | |||
There was a problem hiding this comment.
Why there is script here... separate the javascript logic in a js file.
You should not mix language.
Respect "separation of concerns"
| this.fields.splice(index, 1); | ||
| } | ||
| this.fields.splice(index, 1); | ||
| } |
There was a problem hiding this comment.
This file is way too long, extract js, separate the form into more maintainable code. Using blade parts could be an idea
| } | ||
| }; | ||
| } | ||
| </script> |
There was a problem hiding this comment.
This file seems to have massive deduplications with the other one.
Refactor the code to have the logic written only once
2 participants
No description provided.