Skip to content

#2428 bootstrap 5 theme#2478

Closed
wegry wants to merge 2 commits intorjsf-team:masterfrom
wegry:2428-bootstrap-5-theme
Closed

#2428 bootstrap 5 theme#2478
wegry wants to merge 2 commits intorjsf-team:masterfrom
wegry:2428-bootstrap-5-theme

Conversation

@wegry
Copy link
Contributor

@wegry wegry commented Jul 18, 2021

Reasons for making this change

#2428 Add theme support for the latest version of bootstrap@5 and react-bootstrap@2 to take advantage of not needing to support IE 11. This is largely just a fork of the bootstrap-4 theme with react-bootstrap on the later version.

Checklist

  • I'm updating documentation
  • I'm adding or updating code
    • I've added and/or updated tests
    • I've updated docs if needed
  • I'm adding a new feature
    • I've updated the playground with an example use of the feature

@wegry wegry force-pushed the 2428-bootstrap-5-theme branch from 9f68384 to 05fdebf Compare July 18, 2021 21:33
@jimmycallin
Copy link
Collaborator

Good stuff! I'm feeling we might need to start deprecating old bootstrap themes and remove them in a later release. Supporting each major version in a current manner doesn't feel very maintainable. Thoughts, @epicfaace ?

@wegry
Copy link
Contributor Author

wegry commented Jul 22, 2021

@jimmycallin I think migrating @rjsf/core or @rjsf/bootstrap-4 to Bootstrap 5 would be pretty painful upgrade for downstream users.

@rjsf/core vs @rjsf/bootstrap-4 is 7K downloads a week v. 43K. We use yarn zero installs at work, so we don't show up in either number.

I don't think there's been any rumbling about dropping IE 11 in this project, but Bootstrap 3 and 4 support it. Bootstrap 5 (and the way this PR's version of @rjsf/bootstrap-5 build) does not.

@jimmycallin
Copy link
Collaborator

@jimmycallin I think migrating @rjsf/core or @rjsf/bootstrap-4 to Bootstrap 5 would be pretty painful upgrade for downstream users.

@rjsf/core vs @rjsf/bootstrap-4 is 7K downloads a week v. 43K. We use yarn zero installs at work, so we don't show up in either number.

I don't think there's been any rumbling about dropping IE 11 in this project, but Bootstrap 3 and 4 support it. Bootstrap 5 (and the way this PR's version of @rjsf/bootstrap-5 build) does not.

I think your download numbers are mixed up - bootstrap-4 has 3.5K weekly downloads and core has 48.5K. But I take your main point. 👍

@epicfaace
Copy link
Member

epicfaace commented Jul 25, 2021

Yeah, I like the idea of potentially deprecating bootstrap themes, though my main concern is that doing so would probably make it impossible for certain projects to keep up to date with the latest version of rjsf.

If you already have a site built on bootstrap 3, it's not really imperative to upgrade to bootstrap 4, which would be a huge look-and-feel upgrade as well -- UI frameworks aren't like regular packages in this case.

I'd like to see a solution where someone with a site stuck on bootstrap 3 could still take advantage of the latest version of rjsf with all bug fixes, security fixes, etc. without having to refactor their entire site and upgrade to a new version of bootstrap.

At the same time, there is a lot of duplicate code between the bootstrap-4 and bootstrap-5 themes, and this would make it much harder to maintain all themes in the future, so I'm not sure what the optimal solution is...

@epicfaace
Copy link
Member

I'd like to see a solution where someone with a site stuck on bootstrap 3 could still take advantage of the latest version of rjsf with all bug fixes, security fixes, etc. without having to refactor their entire site and upgrade to a new version of bootstrap.

Okay, well, actually it does look like bootstrap 3 has an EOL past which no security upgrades will be added: https://github.com/twbs/release

So a potential solution is to deprecate bootstrap 3 / 4 / etc. based on the EOL provided by the bootstrap maintainers?

@epicfaace
Copy link
Member

Another related question is if it still makes sense to have a package for each bootstrap version, i.e., @rjsf/bootstrap-3, @rjsf/bootstrap-4, etc. It feels a bit cumbersome to have to deprecate each one. Would it be better to just have a single @rjsf/bootstrap package with all currently supported Bootstrap versions included in it?

@wegry wegry closed this Oct 14, 2021
@epicfaace
Copy link
Member

@wegry why did you decide to close this PR?

@wegry
Copy link
Contributor Author

wegry commented Oct 14, 2021 via email

@Lausselloic
Copy link
Contributor

Hello @epicfaace is there a way to re-open this PR and make it landing? If help is needed I could give some work to it

@epicfaace
Copy link
Member

Yes, feel free to branch off of it and make a new PR yourself.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants