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

bootstrap grid/column support #859

Merged
merged 3 commits into from
Nov 15, 2018

Conversation

Jojoshua
Copy link
Collaborator

@Jojoshua Jojoshua commented Nov 7, 2018

There are a lot of requests for column support. Here is my implementation. I haven't tested with all form elements but the ones I needed worked fine.

This introduces the concept of columns using bootstrap grid system via class attributes set on the formBuilder elements. To set this up ....

  1. add a row identifier onto the class i.e row-1
  2. add the bootstrap grid columns i.e col-xs-6

All elements with the same row- identifier will be grouped into the same row
A second row can be added by repeating the same steps but setting a different row- identifier i.e row-2

1
2

Copy link
Owner

@kevinchappell kevinchappell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just need to confirm editor is working as intended. When I enable bootstrap in the demo the fields look a little off.

src/js/layout.js Outdated Show resolved Hide resolved
src/js/form-render.js Outdated Show resolved Hide resolved
src/js/form-render.js Show resolved Hide resolved
src/js/layout.js Show resolved Hide resolved
src/js/form-render.js Show resolved Hide resolved
src/js/form-render.js Outdated Show resolved Hide resolved
@kevinchappell
Copy link
Owner

Awesome PR 👍 . The rendered form looks pretty good but the editor may need some attention, screenshot above. Overall good work though, formBuilder was way overdue for column support.

In the future we should look at improving the UI in the field edit panel and maybe moving row and column info to a special property in the field data object. Special row and column properties would prevent having to parse the field.classlist

Co-Authored-By: Jojoshua <[email protected]>
@Jojoshua
Copy link
Collaborator Author

Jojoshua commented Nov 9, 2018

So yeah I did not know how to approach this with actually showing the preview inside formBuilder itself.

A couple more ideas that could be done...

  • Add a new config flag to use bs-columns. This flag would then automatically apply rows so that every time you added a form field, it went into the last row. It would then force you to drag the element down into a different row if you wanted it elsewhere. Also, could possibly auto set the col- classes via a resize drag....it all depends on how this should be targeted. For developers, adding the classes to the classList is natural and they understand what is happening. Having a separate property for the row- identifier would be appropriate.

@Jojoshua
Copy link
Collaborator Author

Jojoshua commented Nov 9, 2018

Needed to add /^col-(xs|sm|md|lg)-([^\s]+) to match other bootstrap grid classes like col-md-push-2

@kevinchappell
Copy link
Owner

I think this is ready to merge, in my testing I didn't see any issues. The majority of formBuilder users aren't developers and probably won't be adding columns this way but as long as the column functionality doesn't impact their experience there shouldn't be any issue. It would be great to eventually see formBuilder get to Formeo's level of column support but this is a great step in the right direction.
columns

@Jojoshua Jojoshua force-pushed the bootstrap-grid-support branch from 69ed7e1 to c0b0924 Compare November 15, 2018 01:36
@Jojoshua
Copy link
Collaborator Author

I hadn't seen formeo. Why did you make another project that does the same type of thing? The column support looks like a nice UI

@kevinchappell kevinchappell merged commit 8d513eb into kevinchappell:master Nov 15, 2018
@kevinchappell
Copy link
Owner

kevinchappell commented Nov 15, 2018

Why did you make another project that does the same type of thing?

at the time my thinking was that formBuilder needed so much work to get to a point of being able to support columns, conditions, repeatable input groups, control groups, etc. that i might as well start from scratch.

kevinchappell pushed a commit that referenced this pull request Nov 15, 2018
bootstrap grid/column support (#859)
bs grid support

Authored-By: Jojoshua <[email protected]>
kevinchappell pushed a commit that referenced this pull request Nov 15, 2018
# [3.1.0](v3.0.2...v3.1.0) (2018-11-15)

### Features

* **column:** add bootstrap grid/column support ([e9cc23a](e9cc23a)), closes [#859](#859)
pfranza pushed a commit to pfranza/formBuilder that referenced this pull request Nov 21, 2018
bootstrap grid/column support (kevinchappell#859)
bs grid support

Authored-By: Jojoshua <[email protected]>
pfranza pushed a commit to pfranza/formBuilder that referenced this pull request Nov 21, 2018
# [3.1.0](kevinchappell/formBuilder@v3.0.2...v3.1.0) (2018-11-15)

### Features

* **column:** add bootstrap grid/column support ([e9cc23a](kevinchappell@e9cc23a)), closes [kevinchappell#859](kevinchappell#859)
@futureweb
Copy link

futureweb commented Sep 8, 2020

@Jojoshua @kevinchappell - trying this Feature with latest 3.6.1 Version.
But never getting back the grouped Columns when requesting the HTML?

My Test FormData:

[
  {
    "type": "text",
    "required": false,
    "label": "Textfeld",
    "className": "form-control row-1 col-md-6",
    "name": "text-1599480302286",
    "subtype": "text"
  },
  {
    "type": "textarea",
    "required": false,
    "label": "Textbereich",
    "className": "form-control row-1 col-md-6",
    "name": "textarea-1599480303119"
  },
  {
    "type": "text",
    "required": false,
    "label": "Textfeld",
    "className": "form-control row-2 col-md-6",
    "name": "text-1599566007434",
    "subtype": "text"
  },
  {
    "type": "textarea",
    "required": false,
    "label": "Textbereich",
    "className": "form-control row-2 col-md-6",
    "name": "textarea-1599566005067"
  }
]

Rendering like:

var renderedForm = $("<div>");
formRenderOpts = {
     dataType: "json",
     formData: formData
};
renderedForm.formRender({formData});
formHTML = renderedForm.html();
console.log(renderedForm.html());

Output:

<div class="rendered-form">
   <div id="row-row-1" class="row form-inline">
      <div class="formbuilder-text form-group field-text-1599480302286 row-1 col-md-6"><label for="text-1599480302286" class="formbuilder-text-label">Textfeld</label><input type="text" class="form-control" name="text-1599480302286" id="text-1599480302286"></div>
   </div>
   <div id="row-row-1" class="row form-inline">
      <div class="formbuilder-textarea form-group field-textarea-1599480303119 row-1 col-md-6"><label for="textarea-1599480303119" class="formbuilder-textarea-label">Textbereich</label><textarea type="textarea" class="form-control" name="textarea-1599480303119" id="textarea-1599480303119"></textarea></div>
   </div>
   <div id="row-row-2" class="row form-inline">
      <div class="formbuilder-text form-group field-text-1599566007434 row-2 col-md-6"><label for="text-1599566007434" class="formbuilder-text-label">Textfeld</label><input type="text" class="form-control" name="text-1599566007434" id="text-1599566007434"></div>
   </div>
   <div id="row-row-2" class="row form-inline">
      <div class="formbuilder-textarea form-group field-textarea-1599566005067 row-2 col-md-6"><label for="textarea-1599566005067" class="formbuilder-textarea-label">Textbereich</label><textarea type="textarea" class="form-control" name="textarea-1599566005067" id="textarea-1599566005067"></textarea></div>
   </div>
</div>

As one can see - it's not grouping, but duplicating the IDs ... ?!? Hope you can give me a hint on what's wrong here as I would badly need this "Multi Column" Feature ;-)

Thank you, bye from Austria
Andreas

@futureweb
Copy link

futureweb commented Sep 8, 2020

just figured out it's working when using an actual existing DOM Element within the Page:
var fbRender = document.getElementById("fombuilderrender");
insted of a on the fly generated Element?!?
var fbRender = $("<div>");

@Jojoshua
Copy link
Collaborator Author

Jojoshua commented Jan 28, 2022

Why did you make another project that does the same type of thing?

at the time my thinking was that formBuilder needed so much work to get to a point of being able to support columns, conditions, repeatable input groups, control groups, etc. that i might as well start from scratch.

@kevinchappell Reflecting on this again, I think formBuilder codebase is still workable and can support all that but just needs some restructuring to make it feel extensible/clean again. Also, moving the codebase to Typescript would be a huge first step forward.

@kevinchappell
Copy link
Owner

@Jojoshua a couple other reasons were themeability and not requiring jQuery. The number of changes required to support all of those features seemed like too much for me to handle alone, but with the support of you and the community I think its all doable. I agree there is still a need for formBuilder to continue to grow and moving to typescript would be great.

@Jojoshua
Copy link
Collaborator Author

@Jojoshua a couple other reasons were themeability and not requiring jQuery. The number of changes required to support all of those features seemed like too much for me to handle alone, but with the support of you and the community I think its all doable. I agree there is still a need for formBuilder to continue to grow and moving to typescript would be great.

Just FYI @kevinchappell I was playing around and did a TypeScript PoC. Not exactly sure how to do the webpack properly to build but got rid of all TS errors except for 8 on the formbuilder instance stuff which not really sure how to resolve. https://github.com/Jojoshua/formBuilder/tree/TypeScriptMigrationPoC

@Obaidraj
Copy link

Obaidraj commented Feb 1, 2022

issue1234

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

Successfully merging this pull request may close these issues.

4 participants