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

Easier Bootstrap Grid usage and basic BS4 compatibility #623

Closed
wants to merge 9 commits into from

Conversation

SampsonCrowley
Copy link
Contributor

@SampsonCrowley SampsonCrowley commented Jun 29, 2017

Reasons for making this change

I really would like to be able to use the almost stock react-jsonschema-form, but there just needs to be a way to play nice with bootstrap.

since we already lean more towards bootstrap than material, I added a custom "object" option in the schema to produce bootstrap rows inside the normal object fieldset

Checklist

  • I'm updating documentation
    • I've checked the rendering of the Markdown text I've added
    • If I'm adding a new section, I've updated the Table of Content
  • I'm adding or updating code
    • I've added and/or updated tests
    • I've updated docs if needed
    • I've run npm run cs-format on my branch to conform my code to prettier coding style
  • I'm adding a new feature
    • I've updated the playground with an example use of the feature

@SampsonCrowley
Copy link
Contributor Author

SampsonCrowley commented Jun 29, 2017

Starting with io.js 3 and Node.js 4, building native extensions requires C++11-compatible compiler, which seems unavailable on this VM. Please read https://docs.travis-ci.com/user/languages/javascript-with-nodejs#Node.js-v4-(or-io.js-v3)-compiler-requirements.

The build failing has nothing to do with my code

@SampsonCrowley
Copy link
Contributor Author

I did run cs-check and everything passed on my PC, so please don't not use this PR just because of that

@olzraiti olzraiti mentioned this pull request Aug 1, 2017
9 tasks
@troyconquer
Copy link

fwiw, I'd appreciate basic BS4 support too. Thanks for the work!

@SampsonCrowley
Copy link
Contributor Author

@troyconquer if you're using BS4, i've been using my fork in production without issues. at least until they implement it

@SampsonCrowley
Copy link
Contributor Author

Maybe it's just me, but it seems almost rude to leave all these pull requests open for so long. It's been sitting long enough that I actually don't know if my code is even relevant anymore. why not just close it if your never going to merge it?

I also may just be butthurt that my effort fell on deaf ears and got 0 response from maintainers.

anyways, I'm just gonna close it since I don't really want to put more time into fixing merge conflicts if nothing's gonna happen

@n1k0
Copy link
Collaborator

n1k0 commented Aug 31, 2017

Hey @SampsonCrowley, I'm deeply sorry for the silence here. Just so you know, I've left Mozilla and I won't be writing React code anymore for a little while, so this project is most likely to be actively maintained by other people at Moz, who are very busy on other fronts and have no much time to dedicate supporting use cases they don't have (upgrading to BS4 is one of them as far as I know).

Maybe @glasserc has more information about these aspects.

@troyconquer
Copy link

troyconquer commented Aug 31, 2017

thanks @SampsonCrowley .

Yeah, this lib is great. Would be nice to know a secure maintenance plan @n1k0 .

Have a good day.

@glasserc
Copy link
Contributor

glasserc commented Sep 6, 2017

Hi! Yes, as you have no doubt inferred, the maintainership for this library is quite anemic. If there's anyone who should be maintaining it, it should be me, so I apologize for that. However, I can't really promise that things will be any different going forward. Even when @n1k0 was still on our team, this library already did everything we needed it to -- namely, to fit into kinto-admin and handle the relatively simple schemas we used it for. In other words, this library was mostly in maintenance mode for us, and we didn't envision adding massive new features or breaking new theoretical ground with it. My manager is OK with me spending some amount of time answering issues and reviewing PRs, but we have other priorities too. I try to spend a day each week catching up on rjsf emails but to be honest, Node/JS/React isn't my favorite ecosystem and I've avoided opening those emails for a few weeks now. I'd hoped that another motivated contributor would step up as a maintainer but I understand that my own lackluster participation probably doesn't encourage that. All of which to say, no, I haven't even looked at this PR, for which I'm again sorry.

I noticed that this PR is related to #91, #99, #125, #237, #287, #299, #440, #461, #546, #555, #626, and maybe others, but especially #555. There's been a recurring desire for rjsf to support different UI libraries, but what exactly "support" means hasn't ever really been very clear. It's relatively easy to modify the rjsf code to support whatever library or CSS framework a user has in mind, but coming up with a maintainable API that allows a user to plug in their own CSS framework is a bit harder, and I haven't seen any PRs that do that. Our dependency on Bootstrap right now is relatively light, and I'm hesitant to strengthen that dependency too much while there are clearly users who would rather the use of Bootstrap be completely optional. On top of that, perhaps it's just me, but I'm hesitant to move to Bootstrap 4 until it's out of beta.

I really don't like the idea of a bootstrap type, since it doesn't conform to JSON Schema. I don't think a CSS framework should or even can be defined by the definition of an ObjectField replacement -- how do you handle ArrayField s? what happens if the implementation of ObjectField changes from underneath you? The good news is that if you really do want to use this approach, you don't have to have anything merged upstream, because you can override the ObjectField implementation in your own project. I wouldn't want to merge anything along these lines unless I thought it could support any of the different CSS libraries without my having to do any work. Making something that generic would probably benefit from coordination among all the users who want this feature.

I understand your frustration with having to continually rebase onto master. To be honest, I'm surprised we've even had as many PRs merged as we have. I'm sorry your experience has been so bad and I hope your future experiences in open source are better.

@killua-eu
Copy link

killua-eu commented Sep 20, 2017

A different PoW: we selected react-json-schema for our projects because

  • we lack manpower on our opensource projects on the frontend
  • react-json-schema worked for us really well and we evaluated it as probably the best lib of its kind out there
  • Mozilla was behind this, which we took as a bit of a guarantee that the project won't be abandoned that easily.

With bs4 move to beta and v-4-beta-2 imminent, we wan't to move on to bs4 for several reasons ASAP, so reading through this issue is a bit disappointing. While I understand that @n1k0 and @glasserc can't do too much about it, I'm still looking at you, Mozilla. Seeing you leave a project in a state when it cant accept PRs from guys like @SampsonCrowley doesn't feel right. Mozilla, you are a role model to me and many others out there. We idolize you and look up to you because through you we learned that open source exists. And while I understand you must divest from irrelevant projects at some point, I believe you can do a great job on restructuring them so they can live on and still be really fun. If it's the case of RJF, would you be willing shepherd the project onwards a bit more and gradually open it up to others so that it doesn't just wither away?

To be more specific, @glasserc , is there a way Mozilla could pour some energy at least temporarily to the project to at least have a BS4 branch and BS4 releases? Did you try / is it possible to have more people accept PRs (even if they are outside Mozilla)? While having BS optional would be great, could you still reconsider investing energy into a quick BS4 supporting release to at least create a stepping stone which will ensure this project's relevancy for the feature? Thanks ...

[edited]

@glasserc
Copy link
Contributor

I saw a post from Rémy but I guess he deleted it, which is probably good because it isn't really relevant to what I want to say on the subject.

To make a long story short, I would love to have more contributors to rjsf. I think @olzraiti and @knilink would be excellent candidates for maintainers, for instance. I'm a little anxious about adding almost anyone as a maintainer to the project because Mozilla does still use rjsf (in https://github.com/Kinto/kinto-admin/) and so we have a certain investment in maintaining the status quo. On top of that, I've done a pretty bad job of "growing" random users into contributors and then into maintainers, as I said above (for which I'm sorry).

Part of the problem is that rjsf seems to have attracted a large amount of interest from React developers (which I guess means @n1k0 did something very well) but not all users share the vision we do for this library. I'm only guessing based on the kinds of issues that I see, but many users seem to believe that this library is meant to handle generating any kind of form that you might want in React. (See #641 for a random recent issue of this kind -- not to single that person out, this is representative of a whole class of issues.) rjsf really only cares about automatically generating forms from a JSON Schema, possibly with some additional UI-specific information.

Another part is that even with this stated objective, rjsf falls quite short -- there are some major JSON Schema functionalities that we never got to implement and are kind of shameful. (#52 is the one I'm thinking of at the moment.) So even if you do share our vision, there's a good chance you won't build on rjsf because we don't support the things you need.

Finally, I feel that actually changing rjsf at this point is quite challenging. I'm not sure exactly why this should be -- maybe rjsf has grown too complex, or maybe the JS ecosystem is too complicated, or maybe React is simply too difficult to write something generic in. The net effect is that some contributors show up and start a PR that has a lot of potential, but when I review it seriously, there are problems with it, and perhaps the contributor gets discouraged or distracted, but the PR never gets finished. #417, #682, and #659 are good examples of this category. This is unfortunate and doesn't help grow the rjsf "community".

I would love to have ideas on how to fix all of the above, but again, I can only do so much on this project each week. And, more generally, Mozilla has limited resources too and has to prioritize its efforts to protect the free and open web.

As for supporting Bootstrap 4, upgrading a major version of Bootstrap is kind of a breaking change in this library. If we are going to make such a breaking change, I'm not sure why Bootstrap 4 should be the library we should go with. We have issues requesting support for ant, Foundation 6, and Material. Why shouldn't we go with one of those instead? It seems like this is a decision that some rjsf users want to be able to take on their own. As a result, what I really want to see is a PR that adds "pluggability" to the UI library that rjsf uses, so that other people can maintain Ant, Foundation 6, Material, Bootstrap 4, or whatever support without having to bother us, and users can choose according to their projects to use any of the above. But nobody has stepped up to write such a PR yet, and like I said, I can't really spare the time right now.

I know this doesn't really lead to an encouraging conclusion, but what I really want you, dear reader, to take away from this is that this project is a great place to get involved. I'm extremely motivated to help developers achieve maintainer-level competence and responsibility, although my time is limited. There are well-known gaps in functionality that could use your clever solutions. Tons of developers would use your code. And isn't that enough reward for anybody? 🤣

@danutc
Copy link

danutc commented Sep 23, 2017

One option would be for Mozilla to keep a stable maintenance branch on rjsf project.

For example there could be version 1.x maintenance branch and new development goes to master as version 2.x, 3.x etc. Mozilla has the option to adopt version 2.x / 3.x if it is beneficial for them.

@killua-eu
Copy link

@glasserc as for BS4, Foundation, Ant and Material ... instead of trying to figure out an API, wouldn't branching/splitting the project be the easier, albeit not very elegant way to go? As for the case for BS4: While ant, Foundation and Material are great and IMO render more beautiful output, I personally always default to BS because its the defacto standard. Even without any meaningful js/css skills, I can couple BS with most libraries out there and prototype quickly something meaningful and consistent looking.

@olzraiti
Copy link
Contributor

olzraiti commented Sep 24, 2017

As for supporting Bootstrap 4, upgrading a major version of Bootstrap is kind of a breaking change in this library. If we are going to make such a breaking change, I'm not sure why Bootstrap 4 should be the library we should go with. We have issues requesting support for ant, Foundation 6, and Material. Why shouldn't we go with one of those instead? It seems like this is a decision that some rjsf users want to be able to take on their own. As a result, what I really want to see is a PR that adds "pluggability" to the UI library that rjsf uses, so that other people can maintain Ant, Foundation 6, Material, Bootstrap 4, or whatever support without having to bother us, and users can choose according to their projects to use any of the above. But nobody has stepped up to write such a PR yet, and like I said, I can't really spare the time right now.

I agree here, upgrading to Bootstrap would be a major breaking change (for me, for instance :). A plugin feature would be great. Fortunately, we already have that feature. After(/If?) #653 gets merged (will likely finish that PR this week), all HTML hard coded stuff can be overridden. We could either provide the themes in this library or ask people to create plugin repositories.

If we provided the themes, it could look something like this:

[src/components/Form.js]

...

    const themes = {
        bs4: {
            widgets: [
                ...[<bs4 specific widgets>],
                ...props.widgets // These overriding arrays will default to [] - you know the drill. I'm trying to make this look simple here. They are needed to make the widgets/fields/templates overridable even when we have a theme.
             ],
            fields: [
                ...[<bs4 specific fields>],
                ...props.fields 
             ],
            TemplateField: props.TemplateField || <bs4 specific field template>, // Pass overriding template or use the themed one
            ArrayFieldTemplate: props.ArrayFieldTemplate || <bs4 specific array field template>,
            ObjectFieldTemplate: props.ObjectFieldTemplate || <bs4 specific object field template>
        },
        <other themes>: {
            ...you get the idea
        },
        
    }

    <Form
        {...props}
        {...themes[props.theme]} 
    />
}

If we encouraged users to create plugin repositories, they would be very easy to write in similar style:

import Form from 'react-jsonschema-form';

    // Form with BS4 style
    <Form
        {...props}
        widgets={[
            ...[<bs4 specific widgets>],
            ...props.widgets
        ]}
        fields={[
            ...[<bs4 specific fields>],
            ...props.fields 
        ]}
        TemplateField={props.TemplateField || <bs4 specific field template>},
        ArrayFieldTemplate={props.ArrayFieldTemplate || <bs4 specific array field template>},
        ObjectFieldTemplate={props.ObjectFieldTemplate || <bs4 specific object field template>}
    />

Both look pretty simple to write and maintain, yes?

Having the themes in this lib repo has the benefit that they would be curated by seasoned developers who built this lib and/or are maintaining it. That's also the biggest downside, since this lib is apparently having troubles with maintenance resources. For that reason alone if I had to vote, I would vote for encouraging people to create plugin repositories.

EDIT: After further consideration, I'm not sure if overriding fields/widgets/templates is a powerful method enough for a proper theming. If some widget needs to be themed - and just wrapping the default widget with a div with a class name isn't enough - the whole widget must be copy pasted and modified... I'm not sure if there are many such cases, but we'd have to extend the template system possibly to cover more fields/widgets if that is the case. This could potentially require quite a lot work and there could be a better way to achieve theming than the course I'm proposing. I'm hoping just wrapping the default widgets would be enough?

EDIT2: Fix pseudo code bugs

@killua-eu
Copy link

killua-eu commented Sep 26, 2017

@olzraiti, awesome! I'm in no means an expert on css frameworks, but I believe that overriding classes should be enough as well.

@glasserc
Copy link
Contributor

[In order to support both Bootstrap 3 and Bootstrap 4,] One option would be for Mozilla to keep a stable maintenance branch on rjsf project.

That adds to our maintenance burden so I don't think it's likely.

as for BS4, Foundation, Ant and Material ... instead of trying to figure out an API, wouldn't branching/splitting the project be the easier, albeit not very elegant way to go?

Anyone can already do this, and I think some actually do! In #626, there's a link to someone's ant-design based repo. But of course, as our project evolves, fixes bugs, adds features etc., any fork does not necessarily update along with it.

After further consideration, I'm not sure if overriding fields/widgets/templates is a powerful method enough for a proper theming.

I agree, but so far we haven't had any contributor ambitious enough to come forward with a proposal for what they actually need.

@SampsonCrowley
Copy link
Contributor Author

SampsonCrowley commented Oct 3, 2017 via email

@jwickens
Copy link

Hey guys just wanted to say i think bootstrap 4 support sounds great. Sorry to hear that you're having some maintainer issues, but thanks everyone for pushing forwards with the discussion.

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.

None yet

8 participants