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

List/Detail/Form Field Lists #18

Open
djpeacher opened this issue Jun 12, 2023 · 10 comments
Open

List/Detail/Form Field Lists #18

djpeacher opened this issue Jun 12, 2023 · 10 comments
Labels
documentation Improvements or additions to documentation

Comments

@djpeacher
Copy link

Hi Carlton! 👋🏼 Playing around with Neapolitan 🍨 and I'm wanting to define specific fields lists for the list/detail/form templates. For example, something like:

class RecipeView(CRUDView):
    list_fields = ["name"]
    detail_fields = ["description"]
    form_fields = ["name", "description"]

Played around with this locally, and seemed to work fine. These could also be optional, with fields as the default/fallback. Could maybe add get_X_fields() hooks in addition or instead.

Anyway, just something I noticed while tinkering with it today, and wanted to share! Let me know what you think ✌🏼

@carltongibson
Copy link
Owner

carltongibson commented Jun 13, 2023

Hi @djpeacher. Thanks for giving it a run!

There's a question of how much API to add here. At some point — maybe here — just override the template becomes the advice. Prior to that I'm much more likely to point to (say) doing something in get_context_data() (adding the appropriate vars to the template) rather than a whole load of little get_X_fields() hooks.

For the form, I'm likely to say override get_form_class(), rather than adding an additional attribute.

And so on...

The point being, I think there's already plenty of API available.

Does that make sense?

Let me ponder. I shall mark this as a documentation ticket for now. It might be a good topic for a How To.

@carltongibson carltongibson added the documentation Improvements or additions to documentation label Jun 13, 2023
@djpeacher
Copy link
Author

I see, that makes sense! I understand not wanting to bloat the API, and to your point, there is enough already that I am able to achieve this behavior with some overrides 👌🏼 so yes, perhaps this is best as a How To 🤔

Thanks for considering! 🙏🏼

@kasun
Copy link
Contributor

kasun commented Jun 27, 2023

I'm voting to have separate fields, ex: detail_fields and functions, ex: get_x_fields() at-least to customize the detail view page if possible.

I feel if we have to write a How to for a very common feature then it's not straight forward enough.

No pressure though, I get that you don't want to bloat the View class. Just wanted to put forward my view just in case if you are still deciding about this.

Thanks.

@carltongibson
Copy link
Owner

I am still considering this, but there's almost zero chance I'll want to add as much API as that. Too many hooks...

@djpeacher
Copy link
Author

djpeacher commented Jun 27, 2023

FWIW, my solve for this was to override self.fields on the appropriate view, which I actually don't mind! Perhaps not as clean as an attribute, but it does have a nice LoB feeling to it.

def list(self, request, *args, **kwargs):
    self.fields = [...]
    return super().list(request, *args, **kwargs)

def detail(self, request, *args, **kwargs):
    self.fields = [...]
    return super().detail(request, *args, **kwargs)

def get_form(self, request, *args, **kwargs):
    self.fields = [...]
    return super().get_form(request, *args, **kwargs)

@carltongibson
Copy link
Owner

That also fits the kind of Don't be scared to override the handlers vibe that I'm after too. 🕺🏿

@joshuadavidthomas
Copy link
Contributor

@jefftriplett implemented this in our fork -- westerveltco#6. (There's also some filterset changes in there, related to westerveltco#12.)

This allows one to set either a list_fields and/or detail_fields class attribute on a CRUDView to adjust the fields rendered for a list or detail view.

@carltongibson
Copy link
Owner

I'm still thinking about this.

Assuming we want to use it for display and list purposes as well as form generation, in essence field is dependent on the active CRUD role. So, I'm pondering what API might look like there.

It could well be that Just put an attribute on the class in fact is the best approach there, but I'm not quite ready to concede that yet.

@jefftriplett
Copy link

My workaround has been peppering my views with:

    list_fields = [...]

    ...

    def list(self, request, *args, **kwargs):
        self.fields = self.list_fields
        return super().list(request, *args, **kwargs)

@carltongibson
Copy link
Owner

Yeah... That's not dissimilar to @djpeacher's workaround.

Both of which I'm like 👍 for.

I haven't closed this because I see the desire, but I'm only going to add things I'm sure about.

This is top-level API. I'm likely to pick away at the lower bits before resolving this finally.

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

No branches or pull requests

5 participants