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 fieldsets collapsed default value option. #173

Closed
fabiocaccamo opened this issue Sep 27, 2022 · 12 comments · Fixed by #177
Closed

Add fieldsets collapsed default value option. #173

fabiocaccamo opened this issue Sep 27, 2022 · 12 comments · Fixed by #177
Assignees
Labels
enhancement New feature or request

Comments

@fabiocaccamo
Copy link
Owner

fabiocaccamo commented Sep 27, 2022

Ideas:

@fabiocaccamo fabiocaccamo added the enhancement New feature or request label Sep 27, 2022
@fabiocaccamo fabiocaccamo self-assigned this Sep 27, 2022
@merwok
Copy link
Contributor

merwok commented Sep 27, 2022

I am using the collapse class provided by django in one admin class of one project, with a custom script to expand the fieldset as initial state (in other words, I need the fieldsets collapsible, not collapsed). I add a class show to the fieldsets that need it so that the script can look for it (it could be collapse-initial-show to be more explicit).

Recently the custom script stopped working, so after fighting a bit with DOM event collapsing and bubbling (ugh) I decided to copy django’s collapse.js to my project and change the behaviour directly. (This would also avoid the visual issue of fieldset being visible, then django’s script loads and collapses them, then my script expands them — now it will just add the buttons.)

So, what if django-admin-interface copied static/admin/js/collapse.js from django to override it?
Would be great if I could help get this feature in an upstream project!

@fabiocaccamo
Copy link
Owner Author

fabiocaccamo commented Sep 27, 2022

That would be great, and since there is JS involved for doing this I would add also:

👉 Storing locally each fieldset state (per model class), the cache key could be f"{app_name}-{model_name}-{fieldset_name}". #160

Could you work on this feature?

@merwok
Copy link
Contributor

merwok commented Sep 27, 2022

I’ll make a PR with the django script modified.

I will look at per-user state persistence in a separate commit. Is there an easy way to get the theme option from JS? Output it as a data attribute from a template?

@fabiocaccamo
Copy link
Owner Author

fabiocaccamo commented Sep 27, 2022

I don't think we need a per-user state persistence, I would store them using local storage (already did a similar thing for foldable apps feature).

Exposing theme options as JS variables could be good and useful thing.

@merwok
Copy link
Contributor

merwok commented Sep 27, 2022

I meant that local storage is per-browser persistence, not adding a model with foreign key to user!

@merwok
Copy link
Contributor

merwok commented Sep 27, 2022

Exposing theme options as JS variables could be good and useful thing.

I found the current technique in a template:

{% if theme.foldable_apps %} foldable-apps {% endif %}

Easy to use the same.

@fabiocaccamo
Copy link
Owner Author

fabiocaccamo commented Sep 28, 2022

I meant that local storage is per-browser persistence, not adding a model with foreign key to user!

True, but how often different users will use the same backend with the same computer/browser?
Considering also that we are not storing sensitive data, but just an accordion state...


I found the current technique in a template: ...

Yes, body classes are perfect for features on/off, no need to expose variables.

@merwok
Copy link
Contributor

merwok commented Sep 28, 2022

I don’t understand this sub-part of the discussion!

You requested to persist current collapsed state in local storage. I said I don’t need that for my project but will add it.
Then I called that per-user persistence by mistake instead of per-browser, I wasn’t making a different proposal 🙂

@fabiocaccamo
Copy link
Owner Author

fabiocaccamo commented Sep 28, 2022

Ahh ok, now I understand, sorry.

@fabiocaccamo
Copy link
Owner Author

@merwok have you planned to work on #160 in the next days?

I ask it to decide if publishing a new release with the latest features/bug-fixies today/tomorrow or waiting for this feature too.

@merwok
Copy link
Contributor

merwok commented Oct 6, 2022

If it’s very easy to make a release, then you could ship what’s already done now, then another one soon including the changes for #160 and #174 . If it’s trouble, then you could wait to release next week with everything.

@fabiocaccamo
Copy link
Owner Author

Released 0.21.0 version.

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

Successfully merging a pull request may close this issue.

2 participants