-
Notifications
You must be signed in to change notification settings - Fork 147
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
Add BS5 support #350
Add BS5 support #350
Conversation
`sr-only` has been renamed to `visually-hidden`.
`data-toggle` has been renamed to `data-bs-toggle`.
`data-dismiss` has been renamed to `data-bs-dismiss`.
`.input-group-append` and `.input-group-prepend` have been removed, addons now need to be placed on the same level as the inputs.
Labels now require a `form-label` class.
The `form-inline` class has been removed.
The `form-inline` class has been removed.
The `form-group` class has been removed.
Codecov Report
@@ Coverage Diff @@
## bs5 #350 +/- ##
============================================
+ Coverage 99.25% 99.29% +0.04%
- Complexity 329 332 +3
============================================
Files 21 21
Lines 934 994 +60
============================================
+ Hits 927 987 +60
Misses 7 7
Continue to review full report at Codecov.
|
I've also updated my UI test app for this, however somehow composer isn't able to require commit hashes anymore for me, but that's a problem for tomorrow. edit: OK, alternatively, for those who reeeaaally want to use it for having a quick look at the output (it now also comes with views for bake templates and layouts, so it's easy to spot what's broken there), in step 2, do this instead: composer config repositories.bui vcs https://github.com/ndm2/bootstrap-ui
composer require --no-update friendsofcake/bootstrap-ui:dev-bs5-support |
The flexbox attempt for inline forms wasn't the proper way, so if anyone's looking at it, just ignore that for now, I'll try to fix that next using rows/cols. |
It's more readable, and fixes CS line length violation.
So I've decided to go for auto-sizing for inline forms, using columns that stack based on window size felt wrong. However I'm not going to check off that point, as people might have thoughts about this solution. |
Adapts HTML and CSS according to the updated Bootstrap 5.0 examples.
Furthermore this also enables rendering the error messages for inline checkboxes/switches as tooltips. To achieve the same behavior as in earlier versions, rendering errors can be disabled via control options.
Furthermore this change makes it possible to add custom classes to help text elements via options.
The other night I added autocomplete for bootstrap icons - see PR #350 for FormatHelper::icon() I bet the HtmlHelper::icon() in this repo could also benefit from some kind of IdeHelper task that provides optional autocomplete for IDEs like PHPStorm on those magic strings (icon names). |
I'm not gonna stop anyone from pushing a PR 😛 |
Makes the list of column sizes a numerically indexed array, avoiding assumptions about the number of possible columns. Abandons the `right`/third column config altogether, as it was never actually used.
Typehints, formatting, unused code, etc...
Unifies ARIA usage for all controls, and adds support for specifying ARIA label text via options, making it possible to avoid using custom templates. Also makes the core methods respect the helper configuration, and as a side effect adds support for custom templates via options for the `first()`/`last()` methods.
Some fixes, a little restructuring, adds new sections so that most features are documented.
So that's it for now, I'm not going push further changes (except after review feedback of course). If anyone wants to review this big pile of changes in peace - now's your chance :) |
Co-authored-by: Mark Sch. <dereuromark@users.noreply.github.com>
So if there aren't any objections I'd merge this soon, and then maybe we can release an alpha/preview to possibly garner some feedback. |
So here we go... as requested, I'll tag you now that it's merged @ADmad 😛 |
@ndm2 Do you need me to actually do anything? 🙂 You can start the alpha/beta as you see fit. |
Not really, I guess... you just said "tag me when it's merged" :) If you're cool with it, then I'll draft an alpha pre-release. |
Did you want to try to make a PR for the sandbox, to showcase some live demo cases of it? |
Maybe later when things are finalized, I'm not familiar with the Sandbox app yet. |
As hinted in #349, this PR isn't finished yet, depending on the feedback I'd resolve additional outstanding points and add to this PR.
That being, said, what's already included, is ready to be reviewed. It looks like a lot, but the largest part of the changes is in the tests, and that's mostly a lot of small snippets, like added/removed/moved classes and slight markup structure changes.