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

Update xoopsload.php #668

Open
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
2 participants
@tad0616
Copy link
Contributor

commented Apr 4, 2019

Modify the loadCoreConfig() function to add xoopsformrendererbootstrap4

Update xoopsload.php
Modify the loadCoreConfig() function to add xoopsformrendererbootstrap4

@mambax7 mambax7 requested a review from geekwright Apr 4, 2019

@geekwright

This comment has been minimized.

Copy link
Contributor

commented Apr 5, 2019

This PR and #667 really should be combined. Not a big problem, just neither stands alone.

What we really need to go with this is a reference Bootstrap4 based theme for XOOPS. Both of the currently included form renderers have such a reference, 'default' for legacy and 'xBootstrap' for bootstrap3. This makes it practical to test each renderer against XOOPS internals and a range of common modules. Without that reference theme, both #667 and #668 won't go.

I know there are several v4 themes, including an xBootstrap version that @mambax7 has been working on. That might be a possibility, or maybe there is something new and innovative? We need to decide what this new theme will be.

In the meantime, the renderer can still be used without core changes. One of the design goals of the form renderer concept is that it can be included in the theme itself, and can be customized as needed. There are a lot more css/js frameworks than just 2 bootstrap variations, and most of them will never get a pre-built renderer.

I do think there is great value in adding Bootstrap 4 support. We just need to decide on the reference implementation.

Also note, 2.5.10 is well into its release cycle. There are already modules waiting for it. The changes discussed here will not be merged until the current release cycle is completed. The target would be 2.5.11.

@geekwright geekwright added this to the 2.5.11 milestone Apr 30, 2019

@mambax7 mambax7 referenced this pull request Apr 30, 2019

Merged

Formrender #588

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.