Skip to content
This repository has been archived by the owner on Feb 1, 2022. It is now read-only.

My go at adding (some) Bootstrap 4 support #410

Closed
wants to merge 37 commits into from
Closed

Conversation

Herst
Copy link
Collaborator

@Herst Herst commented Sep 5, 2017

Work in progress, early state ATM.

Note that this is a separate version just for BS4, so this pull request should not actually go into master (at least until there is a separate branch for BS3).

@Herst Herst changed the title My attempt at some BS4 compatibility My attempt at some BS4 support Sep 5, 2017
@Herst Herst changed the title My attempt at some BS4 support My go at (some) BS4 support Sep 5, 2017
@Herst
Copy link
Collaborator Author

Herst commented Sep 6, 2017

Classes which appear to be in bootlint.js but not in (v4's) bootstrap.css which are to be removed/renamed:

  • .carousel-control
  • .checkbox
  • .checkbox-inline
  • .form-horizontal
  • .glyphicon
  • .hide
  • .img-responsive
  • .item
  • .media-left
  • .media-right
  • .navbar-btn
  • .navbar-left
  • .navbar-right
  • .panel
  • .panel-body
  • .panel-collapse
  • .panel-footer
  • .panel-heading
  • .panel-title
  • .pull-left
  • .pull-right
  • .radio
  • .radio-inline

@Herst
Copy link
Collaborator Author

Herst commented Sep 6, 2017

Some (incomplete) lists:

Warnings/Errors to be removed

  • W002 (X-UA-Compatible)
  • W010 (media pulls)
  • W012 (.container required to be first child of .navbar)
  • E002 (Bootstrap v2 .spans)
  • E004 (nesting of containers)
  • E015 (Having multiple add-ons on a single side of an input group)
  • E027 (.table-responsive on outside div)
  • E030-E031 (glyphicon stuff)
  • E040 (Bootstrap v2 .modal.hide)

Warnings/Errors in need to be adapted/redone

  • W009 ("Using empty spacer columns isn't necessary") [no more offset classes]
  • W014 (carousel stuff) [e.g. .carousel-inner > .item.carousel-item]
  • E011 [form-row]
  • E013 [form-row and support for e.g. w-100]
  • E014 [form-row]
  • E017-E020 (checkbox/radio classes)
  • E023-E026 panel stuff (now cards)
  • E029 (Redundant column classes) [still need to confirm that it really works all the time → tests]
  • E035 (.form-horizontal or .form-inline on .form-group) [remove .form-horizontal]
  • E037 (Invalid column widths)
  • E039, E043 [navbar, TODO check for .navbar-expand-* as well]
  • E045 [rename .img-responsive]
  • E051 pull-left/pull-right

Warnings/Errors to be added

  • BS3 classes and such which have been removed/reworked [current solution is quite quick-and-dirtily just checking for the occurrence of BS3-specific classes, TODO: better split it up] → 24bc99b
  • W008 for input-groups?
  • E037 improved so that it also checks for too large column widths and understands -auto as well
  • Nested tables are not supported any more.
  • An explicit class, .breadcrumb-item, is now required on the descendants of .breadcrumbs
  • Pagination
  • Badges are no longer floated automatically in list groups and other components. Utility classes are now required for that.
  • .form-check-* stuff
More specific warnings about BS3 classes being used and what their replacements are
  • .list-inline > li.list-inline-item
  • .center-block.mx-auto
  • .img-responsive.img-fluid
  • .table-condensed.table-sm
  • .pull-*.float-*

For others see: http://upgrade-bootstrap.bootply.com/

Far away goals

@Herst
Copy link
Collaborator Author

Herst commented Sep 7, 2017

Updating dist now as well. Bookmarklet pointing to my version:

javascript:(function(){var s=document.createElement("script");s.onload=function(){bootlint.showLintReportForCurrentDocument([]);};s.src="https://rawgit.com/Herst/bootlint/v4/dist/browser/bootlint.js";document.body.appendChild(s)})();

@Herst
Copy link
Collaborator Author

Herst commented Sep 10, 2017

BTW, collaboration is welcome!

@Herst Herst changed the title My go at (some) BS4 support My go at adding (some) Bootstrap 4 support Sep 14, 2017
@XhmikosR
Copy link
Member

Wow, huge change list!

Not sure how to proceed though. Should we keep compatibility with v3? If so should we make a new branch for v4? Or can we have a common codebase?

@Johann-S @bardiharborow @cvrebert

@Herst
Copy link
Collaborator Author

Herst commented Sep 14, 2017

IMHO, Bootstrap 3 and 4 are too different for the rules (the stuff inside ´src/bootstrap.js´) to be adapted to work with both version which is why in my fork I made it Bootstrap-4-only with a detection for most Bootstrap-3-specific classes (I also threw out everything concerning Bootstrap 2 since I am assuming nobody will switch from v2 to v4).

Of course the rest like the other code files inside src/ could be shared, maybe in combination with the autodetection idea brought up in #304 (comment)

@XhmikosR
Copy link
Member

Can someone chime in on how to proceed with this?

@Johann-S
Copy link
Member

before that PR, bootlint handle Bootstrap 4 Alpha so we can continue to handle BS3 and BS4 if it's possible

@Herst
Copy link
Collaborator Author

Herst commented Oct 24, 2017

FYI, I will have less time for working on this in the next two months.

@rrjanbiah
Copy link

Bump! Looks like an awesome work by @Herst that needs to get into official repo. Can someone from original author(s) check here? TIA

@XhmikosR
Copy link
Member

@mdo: how about you give @Herst push rights to this repo, after you set up branch protection and stuff, so that we get things moving?

@Herst
Copy link
Collaborator Author

Herst commented Jan 21, 2018

Now that the final v4 is out I feel like this should be started anew and done correctly this time, with tests first and PRs for every error/warning to remove/change/add (which then can be separately discussed). I still don't have all the time in the world and this count as a private project so before I begin I would like to assess how much interest there even is for this project and whether there are people willing to give feedback and possibly a helping hand.

It would probably be useful to use out of the the list of recent bug reports those which were the result of wrong usage as a basis for a list of BS4-specific warnings/errors to add (e.g. those related to flex stuff). Some suggestions?

Also, any update on how to handle Bootstrap 3? I would still prefer maintaining separate versions.

@Herst Herst closed this Sep 9, 2018
@Herst Herst deleted the v4 branch September 9, 2018 07:34
@Herst
Copy link
Collaborator Author

Herst commented Sep 9, 2018

I started a clean new attempt with tests at https://github.com/Herst/bootlint-ng, this will be a proper fork with new name (Bootlint-NG instead of Bootlint), with tests and doing it step-by-step.

I am starting with a clean slate with all checks and tests disabled and using Pull Requests for each and every check with gets ported to Bootstrap v4+ (or totally new ones and those being deleted).

@XhmikosR
Copy link
Member

@Herst: would you be interested in having access to the main repo instead? Sorry, I forgot to discuss this sooner with @mdo and the rest, but it seems we lack the manpower to handle this repo too.

@Herst
Copy link
Collaborator Author

Herst commented Sep 14, 2018

@XhmikosR From what I understand there was still no decision on what would happen with Bootstrap 3 support. Since I intent to drop all support for it and only concentrate on the newest Bootstrap version, a fork with a different name makes sense in order to not cause confusion (an alternative would be different major numbers).

@XhmikosR
Copy link
Member

@Herst: yeah, initially that was the plan because v3 is still very used. But I guess we could make one last release (maybe after downgrading cheerio since I had some issues there), tag it and move with v4.

The point is if you are willing to work on this, you might as well do it in the main repo :)

@Herst
Copy link
Collaborator Author

Herst commented Sep 15, 2018

@XhmikosR What about the wiki pages though?
I think I stick to my fork for now and we will see later.

@XhmikosR
Copy link
Member

I don't see the where's the issue. The wiki can be adapted as needed obviously.

But anyway, it's clear so far that you want your "own" fork.

@Herst
Copy link
Collaborator Author

Herst commented Sep 15, 2018

@XhmikosR

The wiki can be adapted as needed obviously._

But how to separate it between what concerns Bootstrap 3 and what concerns Bootstrap 4+? Separate the IDs? Have two sections per wiki page?

But anyway, it's clear so far that you want your "own" fork.

Not necessarily, but now I started it and I like that it like a clean slate beginning when it comes to issues and pull requests (we could do the same here by simply closing all old issues/PRs like was done in the case of Bootstrap 4). But if you want I could switch back, would be even better if I could then get reviews and feedback from other people, especially those involved in the Bootstrap project.

@XhmikosR
Copy link
Member

We can make a parent page which will hold the new v4 rules or something like that.

Anyway, like I said, where there's will there's a way. I don't see something causing issues if you want to work on v4 here.

@Herst
Copy link
Collaborator Author

Herst commented Sep 15, 2018

We can make a parent page which will hold the new v4 rules or something like that.

Well, the rule might stay the same but the description might change (e.g. because of changed classes), therefore my idea with totally separate IDs. But I now that I think of it this might be an overkill, simply a separate section or an explanation what changed with v4+ should suffice.

OK, so let's do it?

@XhmikosR
Copy link
Member

Perfect, give me a couple of days (due to time difference and weekend) to discuss this with @mdo and make a new GitHub team. If you could also jump on our Slack that would be perfect for quick chats too.

@rrjanbiah
Copy link

Updating dist now as well. Bookmarklet pointing to my version:

javascript:(function(){var s=document.createElement("script");s.onload=function(){bootlint.showLintReportForCurrentDocument([]);};s.src="https://rawgit.com/Herst/bootlint/v4/dist/browser/bootlint.js";document.body.appendChild(s)})();

@Herst FYI This v4 bookmarklet doesn't work anymore due to GitHub blocking.

@Herst
Copy link
Collaborator Author

Herst commented Nov 7, 2018

@rrjanbiah
I am not working on that fork any more, instead I have branches in this project.

@Herst
Copy link
Collaborator Author

Herst commented Jul 31, 2019

Updated Bootmarklet for the "next" branch where I am currently doing my work on:

javascript:(function(){var s=document.createElement("script");s.onload=function(){bootlint.showLintReportForCurrentDocument([]);};s.src="https://cdn.jsdelivr.net/gh/twbs/bootlint@next/dist/browser/bootlint.js";document.body.appendChild(s)})();

See #431 for the current progress when it comes to porting existing checks, including what is still missing and where someone could help me.

(/edit: Fixed link. Note: not for excessive traffic.)

@Herst Herst removed the request for review from XhmikosR August 1, 2019 09:19
@rrjanbiah
Copy link

@Herst

You may want to use https://cdn.jsdelivr.net/gh/twbs/bootlint@next/dist/browser/bootlint.js instead of https://raw.githack.com/twbs/bootlint/next/dist/browser/bootlint.js due to CORS

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

Successfully merging this pull request may close these issues.

4 participants