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

feat(table): BS V4.beta.2 New responsive breakpoints and table-dark class #1222

Merged
merged 16 commits into from Oct 23, 2017

Conversation

Projects
None yet
3 participants
@tmorehouse
Member

tmorehouse commented Oct 22, 2017

Bootstrap-V4-beta.2 adds responsive breakpoint support (.table-responsive-{sm|md|lg|xl}), and changes class table-inverse to table-dark for "inverse" tables.

For backwards compatibility, the inverse prop is still available, but the prop dark deprecates inverse.

Addresses issue #747

fix(table): New responive breakpoints and table-dark class
V4.beta.2 adds responsive breakpoint support, and changes class `table-inverse` to `table-dark`

tmorehouse added some commits Oct 22, 2017

@codecov-io

This comment has been minimized.

codecov-io commented Oct 22, 2017

Codecov Report

Merging #1222 into dev will increase coverage by 0.01%.
The diff coverage is 44.44%.

Impacted file tree graph

@@            Coverage Diff             @@
##              dev    #1222      +/-   ##
==========================================
+ Coverage   32.91%   32.92%   +0.01%     
==========================================
  Files         109      109              
  Lines        2865     2870       +5     
  Branches      890      892       +2     
==========================================
+ Hits          943      945       +2     
- Misses       1546     1548       +2     
- Partials      376      377       +1
Impacted Files Coverage Δ
lib/components/table.vue 68.3% <44.44%> (-0.65%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6568005...18a8a6a. Read the comment docs.

@tmorehouse tmorehouse removed the Status: WIP label Oct 22, 2017

@tmorehouse tmorehouse requested a review from alexsasharegan Oct 22, 2017

@tmorehouse tmorehouse changed the title from [WIP] fix(table): New responive breakpoints and table-dark class to fix(table): New responive breakpoints and table-dark class Oct 22, 2017

@tmorehouse tmorehouse requested review from pi0 and mosinve Oct 22, 2017

@tmorehouse tmorehouse referenced this pull request Oct 22, 2017

Closed

[Informational] Bootstrap V4.beta 1, 2 & 3 ship list #747

45 of 56 tasks complete

@tmorehouse tmorehouse changed the title from fix(table): New responive breakpoints and table-dark class to fix(table): BS V4.beta.2 New responive breakpoints and table-dark class Oct 23, 2017

@tmorehouse tmorehouse changed the title from fix(table): BS V4.beta.2 New responive breakpoints and table-dark class to feat(table): BS V4.beta.2 New responive breakpoints and table-dark class Oct 23, 2017

@tmorehouse

This comment has been minimized.

Member

tmorehouse commented Oct 23, 2017

Should the inverse prop be deprecated and replaced with a dark prop, to better reflect the new class/wording in Bootstrap official?

@mosinve

This comment has been minimized.

Member

mosinve commented Oct 23, 2017

I think we could add deprecation notice in docs and at runtime as usual we do with deprecated things, and after a pair of versions delete it.

tmorehouse added some commits Oct 23, 2017

[table] Deprecated 'inverse' prop for new 'dark' prop
To align more closely with new class name
@@ -407,14 +413,22 @@ export default {
});
},
computed: {
computedDark() {

This comment has been minimized.

@mosinve

mosinve Oct 23, 2017

Member

At carousel-slide more laconic deprecation procedure at img-src and src props definition. And without extra variable.

This comment has been minimized.

@mosinve

mosinve Oct 23, 2017

Member

something like this

dark: {
            // Replaces prop `inverse`
            type: Boolean,
            default() {
                if (this && this.inverse !== null) {
                    // Deprecate inverse
                    warn("b-table: prop 'inverse' has been deprecated. Use 'dark' instead");
                    return this.inverse;
                }
                return false;
            }
        },
        inverse: {
            // Deprecated in v1.0.0.beta.10 in favor of `dark`
            type: Boolean,
            default: null
        },

tmorehouse and others added some commits Oct 23, 2017

tmorehouse and others added some commits Oct 23, 2017

@@ -148,7 +148,7 @@ function recToString(obj) {
return toString(keys(obj).reduce((o, k) => {
// Ignore fields that start with _
if (!/^_/.test(k)) {
if (!'/^_/'.test(k)) {

This comment has been minimized.

@tmorehouse

tmorehouse Oct 23, 2017

Member

Can't have quotes around a literal regex

This comment has been minimized.

@mosinve

mosinve Oct 23, 2017

Member

Yep, my bad english... eslint recommended parens not quotes ))

@@ -148,7 +148,7 @@ function recToString(obj) {
return toString(keys(obj).reduce((o, k) => {
// Ignore fields that start with _
if (!/^_/.test(k)) {
if (!(/^_/).test(k)) {

This comment has been minimized.

@tmorehouse

tmorehouse Oct 23, 2017

Member

Literal RegExpr shouldn't need wrapping in brackets.

If ES Lint is complaining, then try this:

if (!(/^_/.test(k)))

This comment has been minimized.

@mosinve

mosinve Oct 23, 2017

Member

it works good with this if (!(/^_/).test(k)) {

@mosinve mosinve changed the title from feat(table): BS V4.beta.2 New responive breakpoints and table-dark class to feat(table): BS V4.beta.2 New responsive breakpoints and table-dark class Oct 23, 2017

@mosinve

This comment has been minimized.

Member

mosinve commented Oct 23, 2017

As for me, it good now.

@tmorehouse tmorehouse merged commit febdfd1 into dev Oct 23, 2017

2 checks passed

License Compliance License checks passed.
Details
ci/circleci Your tests passed on CircleCI!
Details

@tmorehouse tmorehouse deleted the table/v4beta2 branch Oct 23, 2017

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