clean up grid classes #1707
clean up grid classes #1707
Conversation
openshift-ci-robot
added
approved
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: rhamilto, spadgett The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
| classNames('pf-m-4-col-on-md', 'pf-m-6-col-on-sm'), | ||
| classNames('pf-m-4-col-on-md', 'pf-m-6-col-on-sm'), | ||
| classNames('pf-m-4-col-on-md', 'pf-m-hidden', 'pf-m-visible-on-md'), | ||
| classNames('col-sm-4', 'col-xs-6'), |
There was a problem hiding this comment.
I'm inclined to wonder if there are several exact matches in these tableColumnClasses sets that could be shared. For example, this looks the same as the one in machine-deployment, limit-range, default-resource, cluster-service-plan...
There was a problem hiding this comment.
This is obviously a good step, so I wouldn't hold the PR on it.
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest |
|
since you are still currently using BS3 grid elsewhere, and we are not considering a 12 column flexbox grid in PF4, I think this is fine. However, you can still consider a PF4 CSS Grid in the future. It should work technically the same way (and it seems to, for me). Here is one such example: Maybe you can try this globally in the future... |
IMO, until we decide to remove Boostrap's grid entirely, we should avoid using PF4's grid. Let's use and maintain one solution, not two. |
7 participants
/assign @rhamilto