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

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

Merged
merged 16 commits into from
Oct 23, 2017

Conversation

tmorehouse
Copy link
Member

@tmorehouse 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

V4.beta.2 adds responsive breakpoint support, and changes class `table-inverse` to `table-dark`
@codecov-io
Copy link

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 changed the title [WIP] fix(table): New responive breakpoints and table-dark class fix(table): New responive breakpoints and table-dark class Oct 22, 2017
@tmorehouse tmorehouse changed the title fix(table): New responive breakpoints and table-dark class fix(table): BS V4.beta.2 New responive breakpoints and table-dark class Oct 23, 2017
@tmorehouse tmorehouse changed the title fix(table): BS V4.beta.2 New responive breakpoints and table-dark class feat(table): BS V4.beta.2 New responive breakpoints and table-dark class Oct 23, 2017
@tmorehouse
Copy link
Member Author

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

@mosinve
Copy link
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.

@@ -407,14 +413,22 @@ export default {
});
},
computed: {
computedDark() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
        },

@@ -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)) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't have quotes around a literal regex

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Literal RegExpr shouldn't need wrapping in brackets.

If ES Lint is complaining, then try this:

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

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

mosinve commented Oct 23, 2017

As for me, it good now.

@tmorehouse tmorehouse merged commit febdfd1 into dev Oct 23, 2017
@tmorehouse tmorehouse deleted the table/v4beta2 branch October 23, 2017 16:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants