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

The CSS rules for form elements should be more flexible to accomodate different ways of structuring valid forms #286

Closed
jhung opened this issue Mar 23, 2020 · 5 comments · Fixed by #357
Assignees
Labels
bug Something isn't working
Milestone

Comments

@jhung
Copy link
Contributor

jhung commented Mar 23, 2020

Describe the bug
If a form is constructed in a way other than the <input>...</input><label>...</label> pattern, the CSS rule will not apply. For example, if a radio group is structured <label><input>...</input></label> then it will not appear styled properly.

Expected behavior
The current CSS rule input[type="X"] + label should be relaxed / expanded to allow for different valid form structures.

@jhung jhung added the bug Something isn't working label Mar 23, 2020
@jhung jhung added this to the 1.0.0-alpha.12 milestone Mar 23, 2020
@jhung jhung self-assigned this Mar 23, 2020
@jhung
Copy link
Contributor Author

jhung commented Mar 23, 2020

The issue here is being able to create a custom checkbox by using the ::before pseudo-element. This can be done easily using the rule input + label::before. If the input is nested within the label, it would require using Javascript to detect the changes to the child input to update the parent label's ::before.

One possible way to support the <label><input>...</input></label> structure would be to wrap the label text in a span and use a rule like input[type="checkbox"]:checked + .label-text to properly target and style the custom checkbox / radio.

Example HTML

<label>
   <input type="checkbox" /><span class="label-text">My label</span></input>
</label>

Example CSS

input[type="checkbox"]+ .label-text::before {
  ... styles for default state
}

input[type="checkbox"]:checked + .label-text::before {
  ... styles for checked state
}

@greatislander
Copy link
Collaborator

@jhung Can we mark this as #wontfix and close it?

@greatislander greatislander removed this from the 1.0.0-alpha.12 milestone Mar 25, 2020
@erictheise
Copy link
Member

My pseudo-outsider opinion's that Pinecone should be able to handle valid html, which is what Django and potentially other frameworks generate. My template workaround is having to explicitly loop over the elements, generate tags that mesh with Pinecone's parser, and for now I've lost easy access to other values set in my Form object.

If Pinecone's a general purpose framework then #wontfix doesn't seem right. If it's for internal use only primarily with WordPress then maybe it's a different story.

@greatislander
Copy link
Collaborator

@erictheise Would wrapping the label text as suggested in @jhung's comment work in Django?

@erictheise
Copy link
Member

erictheise commented Mar 26, 2020

@jhung emailed me with these options on the 23rd:

"I took a look at the issue of radios and checkboxes not being styled (see github issue #286) and talked it over with Ned. There are solutions and I have listed the options here in order from simplest to complex.

  1. Change Django’s output to match the expected <input/><label>…</label> rule for Pinecone’s CSS.
  2. Wrap the label text in a <span> with a classname as explained here. This will require adding new CSS rules in Pinecone.
  3. Use Javascript to support changing of the label::before pseudo-element based on the state of the child input.

My suggestion is that we try for option 1, with a goal of eventually supporting option 2 in Pinecone."

I followed his first suggestion which involved adding attributes to RadioSelect in forms.py and adding the loop on lines 47 to 54 of surveys/index.html. I've (hopefully temporarily) lost the ability to display a label for the question the radio buttons apply to.

I should be able to just say {{ role_form }} where that loop is. I think I should even be able to include the radio buttons as part of the {{ user_form }}.

This pattern'll need to be implemented for any questions with checkbox or radio button answers.

The <span> approach would still involve a manual loop for each checkbox or radio button group plus the added styles so I don't see that there's anything to be gained by it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants