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

Add missing fallback themes config for list operation #5230

Merged
merged 1 commit into from
Aug 1, 2023

Conversation

karandatwani92
Copy link
Contributor

Copy link
Contributor

@pxpm pxpm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @karandatwani92 thanks for the PR 🙏

How about the other keys ? I know some of them are exclusive of tabler, so they don't make sense to be in ui. But others like body also throw errors on CRUD pages, as reported in #5229

Is there any reason to left those keys out that I am missing ?

Cheers

@karandatwani92
Copy link
Contributor Author

Hey @karandatwani92 thanks for the PR 🙏

How about the other keys ? I know some of them are exclusive of tabler, so they don't make sense to be in ui. But others like body also throw errors on CRUD pages, as reported in #5229

Is there any reason to left those keys out that I am missing ?

Cheers

Hey Master @pxpm . I considered them already and found only CRUD's list view requires classes.___ attribute. Nowhere else CRUD required any attributes like classes.body, classes.sidebar, classes.header, classes.footer.

Thus, no need to add them. 😬

@pxpm
Copy link
Contributor

pxpm commented Jul 31, 2023

@pxpm pxpm assigned karandatwani92 and unassigned pxpm Jul 31, 2023
@karandatwani92
Copy link
Contributor Author

@pxpm let me explain

Why no need for other attributes?

Themes may ask for n number of attributes it could be body or anything. And theme config should only be responsible for those values which the theme requires. Those values must come up with the theme itself.

What fallback attributes should be covered in ui.php

Only attributes that are required by CRUD, PRO, Dev Tools or any Backpack package which has views, so they could match with the theme's classes by overriding default classes.

Till now I have found, only the CRUD's list operation needs attributes which we are adding here.

@karandatwani92 karandatwani92 assigned pxpm and unassigned karandatwani92 Aug 1, 2023
@pxpm pxpm merged commit 169bdca into main Aug 1, 2023
4 checks passed
@pxpm pxpm deleted the fix-list-table-classes-config branch August 1, 2023 09:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants