-
Notifications
You must be signed in to change notification settings - Fork 274
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
[Content] Add diogogpinto-filament-themes-full-guide.md content and respective art #587
base: main
Are you sure you want to change the base?
[Content] Add diogogpinto-filament-themes-full-guide.md content and respective art #587
Conversation
@saade - this article is cleared to be published on the site. I'm going to review it tonight and tomorrow for grammar, content, etc., but I'd love a second set of eyes on it if you're available. 👍🏻 |
- [Render Hooks on the Official Docs](https://filamentphp.com/docs/3.x/support/render-hooks) - check how you can render custom views almost anywhere in your panels | ||
- [Registering Assets](https://filamentphp.com/docs/3.x/support/assets#registering-javascript-files) - check how you can register custom JS/CSS into your Filament panels | ||
|
||
### The ~wrong~ unrecommended way to customize your theme |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We really should not have this section in my opinion as this approach should basically never be used.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm on the fence about this one. On the one hand, I completely agree that we almost never want this approach to be used. However, on the other hand, by having this here, we're specifically calling out the fact that this is the wrong approach, dissuading people from using it.
I guess it comes down to whether or not this is going to prompt more people to remember that this is an option and use it or if it's going to prevent people from using it because it's explicitly stated not to.
I think I come down more on the side of leave it in to ensure that people know it's the wrong way to do things. My hunch is that the people reading this article are (in general) going to be deep enough in Filament to know that overriding views is an option in Laravel 🤷🏻
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can see both points!
I inserted because sometimes you may want to change specific functionality around whole components that may not necessarily break functionality on updates. My case was the topbar, but later I disabled it and used render hooks. But that approach gave me some headaches around mobile and had to create a whole new mobile trigger for the sidebar - if I had to do it all over again, I would publish the file and monitor each release for breaking changes.
Hey guys, I just checked and replied to some of the comments by @alexandersix. Some general thoughts... I'm not sure if this works best as an article vs. being part of the docs. Specifically because Filament v4 will most likely change the way themes are set up. If we still have this article around by then, I think that's going to confuse a lot of people. Also, even though this article does provide value in guiding users through the process step by step, I wouldn't consider it a "full guide" covering all aspects of theming. What would y'all think about adding parts of this article to the docs if there's details missing there currently? 🙂 |
Hey @zepfietje, Thank you for your feedback! "Full guide" is a stretch, there is still a bit missing (custom JS functionality, render hooks vs vendor files, some other things in my todo), but it was built to respond to the amount of DMs I've been receiving regarding custom theming - more of a starting point to a full layout change. If you have any suggestions on what may be missing, I'm totally happy to dwell on the missing content! I think the visual queues (screenshots) go a long way when it comes to help starters in the process, that's one of the reasons why Dan suggested skipping the docs. I think that whatever benefits the community the most is the way to go! Be it a article or a doc extension, I'm happy it can serve some purpose. Thanks again! |
I think it doesn't really fit with the current docs style, but I understand the concerns around v4. I spoke to Diogo about this originally and he is interested in updating it once that release is out |
Yes, as I've mentioned to @danharrin and @alexandersix, I plan to keep the guide updated in line with new releases, breaking changes, and community feedback. I will review each of Alex's suggestions and incorporate additional content that I believe will enhance this guide further - custom JS functionality, for example, I think can be added to enhance customization. Regarding the "wrong" way of doing things that @zepfietje finds unsuitable for this kind of article: While I believe knowledge is power, I understand that it can potentially cause confusion for less experienced developers. I'll leave it to you to decide whether to retain or remove that section. Thanks guys! |
Totally understand your concern @zepfietje - I think this is a "both and" situation. I think it would be highly beneficial to have a Filament v3 version of a theming guide as an article so that people have a resource to look at if they're interested. I think the style of this particular guide fits in better with our article content, and given that v4 is imminent, I wouldn't want to take too much time and effort adjusting our current documentation to fit @diogogpinto's work here. However, if we're so inclined, I don't think there's any reason why we couldn't use this content to inform small adjustments/additions to the existing v3 documentation if we see fit. However, since things are going to change fairly significantly in v4, once we release the new version, we should have both better documentation on theming in our actual docs and a tutorial for v4. People learn in different ways, and articles are intrinsically more "share-able" than documentation pages, so I think that having both options would be the best course of action. Especially since we have @diogogpinto who is willing to keep the article side of things updated as things grow and change. |
This guide was built with three things in mind:
Be it an article or not, this kind of content is attractive and generates a lot of interest, and it's a content that a lot of educators will explore soon. From the business side of things (I'm business first, dev second) I think it's crucial that Filament offers newcomers content that explores pain points and let users dive deeper. Articles, docs, videos, whatever - producing content that gives newcomers the power and help avoid spamming help channels or users directly is greatly beneficial for the Filament brand and its ecosystem, and it's with that in mind that I encourage the core team to just copy whatever you guys see fit and paste into the docs - as I once was a newbie in Laravel and later Filament, I think this kind of content could have helped me in the past. One thing I usually find is overly technical docs. Reading the plugin example on the docs, and deploying a plugin on my own after just reading one document, gave me the feeling that this particular content could ease some pain for some users. Edited for clarity: this is a small dissertation on why I agree with @alexandersix and what he states in his last paragraph. I must restate, this comes from a genuine place of heart with the only intention to help newcomers. I have already shared my repo with some folks that come to me with some questions, and I think it's helpful. I know nothing about OSS business and its singularities, this is just me rambling. 😅 |
Co-authored-by: Alex Six <[email protected]>
Co-authored-by: Alex Six <[email protected]>
Co-authored-by: Alex Six <[email protected]>
Co-authored-by: Alex Six <[email protected]>
Co-authored-by: Alex Six <[email protected]>
Co-authored-by: Alex Six <[email protected]>
Co-authored-by: Alex Six <[email protected]>
Co-authored-by: Alex Six <[email protected]>
Co-authored-by: Alex Six <[email protected]>
Co-authored-by: Alex Six <[email protected]>
Possible todo before publishing:
I won't explore theme publishing as I don't think it fits the scope Let me know if it's worth pursuing. Thanks guys! |
@diogogpinto regarding your todo list:
Thank you again for being so willing to work with us on this! I genuinely think that this will help a ton of people, so your work will be well, well worth the effort 🎉 |
@alexandersix please lmk when i can proceed with the review, ty! |
Needs #576 merge for adding author
Adds an article on how to create custom themes, that goes beyond the current docs.
It has some github specific MD, that I don't know if it's compatible with the current MD structure, like:
Let me know if any changes are required.
Content approved for publishing by @danharrin