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

ENH: What to do with "Collapse" #471

Closed
Timo-Breumelhof opened this issue Sep 18, 2023 · 9 comments · Fixed by #541
Closed

ENH: What to do with "Collapse" #471

Timo-Breumelhof opened this issue Sep 18, 2023 · 9 comments · Fixed by #541
Assignees
Labels
enhancement New feature or request question Further information is requested review in process
Milestone

Comments

@Timo-Breumelhof
Copy link
Contributor

Please summarize your question in one sentence

The current collapse functionality is difficult to style and looks legacy.

Give a more extended description

We have the [GROUPCOLLAPSE] token.
Currently that loads an image and renders this:

Toggle Group Display

image

I see two options to improve on this:

  • We change that it to a fontawesome icon and add a CSS class for the state. (dcf-open dcf-closed)
    That way it looks better and is stylable without a lot of hacks.

  • We remove support for this and leave it to the Theme builder you add his own script?

Opinions?

@Timo-Breumelhof Timo-Breumelhof added enhancement New feature or request question Further information is requested labels Sep 18, 2023
@Timo-Breumelhof Timo-Breumelhof self-assigned this Sep 18, 2023
@Timo-Breumelhof
Copy link
Contributor Author

Related to #51

@johnhenley
Copy link
Collaborator

I vote for option 1

@johnhenley
Copy link
Collaborator

@Timo-Breumelhof
There are 4 different uses of this:
image

The toggleGroup sets a cookie, which the toggleSection does not.
image

While we can probably do something, should we defer this for now?
My brain is too tired to try and come up with a good solution, which will require tweaking the html, js, and css...

@Timo-Breumelhof
Copy link
Contributor Author

@johnhenley agreed v9

@johnhenley
Copy link
Collaborator

@johnhenley agreed v9

Or hopefully sooner. But not v8

@johnhenley
Copy link
Collaborator

johnhenley commented Sep 22, 2023

@Timo-Breumelhof

There are 4 different uses of this:

image

The toggleGroup sets a cookie, which the toggleSection does not.

image

While we can probably do something, should we defer this for now?

My brain is too tired to try and come up with a good solution, which will require tweaking the html, js, and css...

@Timo-Breumelhof I can try this one if you want. I see it more clearly now.

It needs to be refactored and generalized. Having two functions to essentially do the same thing is just lazy :) and the cookies code should be separated out.

You have a "thing" (a section, a group, quick reply, whatever) that is "collapsible" with an "opened" or "closed" state.

(As an aside, this is where our English falls flat. "Open" can be used as an adjective or a verb. "Close" is a just verb--or an adjective with an entirely different meaning:) "Closed" is the adjective. So I use "opened" which is clearer as the adjective opposite of "closed".)

You open or close the "thing" which changes the icon and updates the cookie. Any "thing" that is collapsible deserves a cookie.

You have a div with dcf-collapsible`` and either dcf-collapsible-openedordcf-collapsible-closed. Inside the div` you have the fontawesome icon tag.

What do you think?

@Timo-Breumelhof
Copy link
Contributor Author

Sounds good to me.
How would you target the target element to collapse / expand?
BTW target the target, nice one too ;-)

@WillStrohl
Copy link
Member

I vote for option 1

Me too.

@johnhenley
Copy link
Collaborator

@Timo-Breumelhof
There are 4 different uses of this:
image
The toggleGroup sets a cookie, which the toggleSection does not.
image
While we can probably do something, should we defer this for now?
My brain is too tired to try and come up with a good solution, which will require tweaking the html, js, and css...

@Timo-Breumelhof I can try this one if you want. I see it more clearly now.

It needs to be refactored and generalized. Having two functions to essentially do the same thing is just lazy :) and the cookies code should be separated out.

You have a "thing" (a section, a group, quick reply, whatever) that is "collapsible" with an "opened" or "closed" state.

(As an aside, this is where our English falls flat. "Open" can be used as an adjective or a verb. "Close" is a just verb--or an adjective with an entirely different meaning:) "Closed" is the adjective. So I use "opened" which is clearer as the adjective opposite of "closed".)

You open or close the "thing" which changes the icon and updates the cookie. Any "thing" that is collapsible deserves a cookie.

You have a div with dcf-collapsible and either ``dcf-collapsible-openedordcf-collapsible-closed`. Inside the `div` you have the fontawesome icon tag.

What do you think?

@Timo-Breumelhof BTW, the cookie doesn't really work. If you load a topic, toggle the quick reply, then refresh the browser, the quick reply is visible again :)

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

Successfully merging a pull request may close this issue.

3 participants