Codecov Report

Merging #2805 into dev will increase coverage by 4.06%.
The diff coverage is 97.47%.

@@            Coverage Diff             @@
##              dev    #2805      +/-   ##
==========================================
+ Coverage   76.93%   80.99%   +4.06%     
==========================================
  Files         174      194      +20     
  Lines        3173     3199      +26     
  Branches      939      941       +2     
==========================================
+ Hits         2441     2591     +150     
+ Misses        532      441      -91     
+ Partials      200      167      -33

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 172d876...9c46ff9. Read the comment docs.

commenting user-select: none; in _table.scss breaks selectable feature (especially range mode), and since it only applies to tables in selectable state, it shouldn't cause issue with the newly added #2801

Hmmm... but user select disables the ability to select text. Maybe this class should only be applied if there is at least one row selected first? and removed when no rows are selected?

Yes, it is the desired behavior of user-select.
I'm not sure of the usability of text selecting, when a table is in selectable state. This state makes the table an active UI object, like a button or a link, something probably not suitable for text selection.
Anyway, while it doesn't matter for single or multi modes, the lack of it in range mode disables shift-click because the recent 'text selected' abort of row-clicked, then making it impossible to do a shift-click range selection.
If you prefer to disable user-select only for range mode, then perhaps we could add a selector indicating the select-mode, so we could use a more specific CSS selector.

@lianee maybe a new class should be added to b-table table element, only when at least one row has been selected (i.e. b-table-selecting') which applies the user-select: none; (or a dynamic style added to all TRs)

 computed: {
    isSelecting() {
      return this.selectable &&
        this.selectMode === 'range' &&
        this.selectedRows &&
        this.selectedRows.some(v => v)
    }
 }

And if this.isSelecting returns true, then the class b-table-selecting is added to the table element.

I guess the 2 classes (selecting and mode) should be added, making it more versatile, then users could override the default behavior just with CSS.

@lianee check out PR #2865

@tmorehouse You were too fast for me ;)
I would have decoupled the two things with a b-table-selecting and a b-table-select-{mode} classes, but I'm ok with your solution, thank you.

We could stilldo so.ethjng similar forthe other two modes (adding classes when section is active).. just no user-select none.

And maybe add classes to say which mode is in use.

I don't know how to suggest changes to your PR, but what I was thinking is:
table.js:

'b-table-selecting': this.isSelecting,
[`b-table-select-${this.selectMode}`]: true

mixin-selectable.js:

    isSelecting() {
      return (
        this.selectable &&
        this.selectedRows &&
        this.selectedRows.some(Boolean)
      )
    }

_table.scss:

  &.b-table-selecting.b-table-select-range {
    > tbody > tr {
      user-select: none;
    }
  }

leaving the user the choice to apply user-select in any mode he wants, whether or not there is a selection in process.

But as I said, your solution works ok, range mode is now working as it should.

Could you put this as a comment on the new PR?

I can add it in later today.