Skip to content
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

Feature enhancements #48

Open
DCox2016 opened this issue Jun 11, 2023 · 4 comments
Open

Feature enhancements #48

DCox2016 opened this issue Jun 11, 2023 · 4 comments

Comments

@DCox2016
Copy link

Hello VentureDrake team,
I created a feature enhancement for the css. This enhancement adds a css.php configure file to Config. folder in the larvel-crm project. Than the configuration uses the env file for its variables to set the css, favicon, and fonts of the app.blade.php. If the env files does not have variables set than it fails back to the defaults. I would like to share the enhancement. How would I support the enhancement to the repo for review ?
Thanks

Here is a screen shot of my enhancement.
image

@andrewdrake
Copy link
Contributor

This is a nice idea.

First you should create this issue in the actually laravel crm package, this project is an example of using that package. I should actually disable issues on this repo for that reason.

Secondly what you want to do is to create a pull request for your enhancement, we would then review it and merge into the master branch.

Now on the topic of being update to change design style, this might be better handled using the settings table, and adding fields to the settings section of the UI. We actually have plans for this already, open for discussing.

@DCox2016
Copy link
Author

I have worked on a project before where the css was store in the database. The project would used 'state'. State does not always work currently and I found I spent a lot of my time logging, debug and tracking done the issue. For me I would prefer to use env variables with a css.php config file. To querying a database to get css values is not a good idea in my opinion. That is just my opinion

Here are some thinking to think of

  • Performance: Querying a database for CSS values for every page load can introduce additional overhead and slow down the rendering process. Retrieving data from a database typically involves network latency and additional processing compared to reading data directly from a file.

  • Maintainability: CSS is typically considered part of the presentation layer and is best kept separate from the business logic and data storage. By storing CSS values in a database, you mix the concerns of data management and presentation, which can make your codebase more complex and harder to maintain.

  • Flexibility and Scalability: CSS files are designed to be easily editable and modular. By storing CSS values in a database, you lose the ability to easily make changes to the stylesheets and take advantage of the browser's caching mechanisms. Additionally, as your project grows, managing a large number of CSS records in a database can become cumbersome and less scalable.

  • Code Readability: Embedding CSS values within database queries can make the code harder to read and understand. Separating HTML, CSS, and JavaScript into their respective files and keeping them together allows for better organization and clarity.

Another thing that my team brought up is how people will use your project. If a stand alone project, yes css in database not that bad. What if someone is using your project as a composer package? That where things get hairy. We are using your project as a composer package. I know that your using Spadie and a permission.php file is made

'teams' => env('LARAVEL_CRM_TEAMS', false),

and I saw this and thought this could work. Please let me know what you think and if I should continue the steps of sending you the code.

@DCox2016
Copy link
Author

Hello VentureDrake team, just checking in. Does your team want to move forward with me submitting the issue and code ? My lead on the project said if your team wanted to schedule a meeting he would be more than happy to answer any questions. Please let me know. Thank you

@andrewdrake
Copy link
Contributor

Yes, but please submit this on the package repo

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants