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

Support .grid + small in forms #271

Merged
merged 1 commit into from
Jan 28, 2023
Merged

Support .grid + small in forms #271

merged 1 commit into from
Jan 28, 2023

Conversation

cmbuckley
Copy link
Contributor

If using a .grid inside a form, a small info field will not have the correct vertical spacing compared to standard input elements.

This change adds .grid to the list, providing it is enabled.

Open to suggestions for a better way to build the selector list and retain the use of :where.

@lucaslarroche
Copy link
Member

lucaslarroche commented Dec 26, 2022

Hi @cmbuckley,
Sorry for the delay. I just looked, and actually, I don't see any problems.

What is your HTML structure?
I tested with:

<form>
  <div class="grid">

    <label for="one">One
      <input id="one" name="one">
      <small>Helper</small>
    </label>

    <label for="two">Two
      <input id="two" name="two">
      <small>Helper</small>
    </label>

    <label for="three">Three
      <input id="three" name="three">
      <small>Helper</small>
    </label>

  </div>
</form>

@lucaslarroche lucaslarroche added the question Further information is requested label Dec 26, 2022
@cmbuckley
Copy link
Contributor Author

cmbuckley commented Dec 26, 2022

Hi,

.grid + small denotes an info field following the grid, which would be used as a helper for all items inside the grid:

<form>
  <div class="grid">
    <label for="one">One
      <input id="one" name="one">
    </label>
    <label for="two">Two
      <input id="two" name="two">
    </label>
  </div>
  <small>Helper that describes One and Two together</small>
</form>

This is particularly useful if the grid describes a key/value pair, for example.

Edit: rebased off latest and removed derived file changes to minimise future conflicts

@lucaslarroche
Copy link
Member

Hi @cmbuckley,

Thank you. I finally correctly tested it. That's a great addition.
I will merge your PR into the dev branch. It will be merged into master with #289.

I like how you managed the $enable-classes condition.
I should do this everywhere in the code.
I added it to the long to-do list for v2. I want to avoid doing significant refactoring on v1.

@lucaslarroche lucaslarroche merged commit 010dc55 into picocss:dev Jan 28, 2023
@lucaslarroche lucaslarroche mentioned this pull request Jan 28, 2023
lucaslarroche added a commit that referenced this pull request Apr 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants