Dropdown toggle caret doesn't respect left/right direction

[silhusk]

The caret in the dropdown toggle is shown pointing downward also when the @direction is left or right. The class being added results in dropleft, resp. dropright. Whereas, for BS5 they should be dropstart, resp. dropend (they respect RTL).

ember-bootstrap/addon/components/bs-dropdown.js

Line 242 in 4ec513d

return this.direction !== 'down' ? `btn-group drop${this.direction}` : 'btn-group';

ember-bootstrap/addon/components/bs-dropdown.js

Line 244 in 4ec513d

return `drop${this.direction}`;

$ npx ember bootstrap:info
Npm packages:
ember-bootstrap: ^6.0.0-1 -> 6.0.0-1
ember-cli: ^4.10.0 -> 4.10.0
bootstrap: ^5.1.1 -> 5.2.3
bootstrap-sass: n/a
ember-cli-sass: ^11.0.1 -> 11.0.1
ember-cli-less: n/a
Bower packages:
bootstrap: n/a
bootstrap-sass: n/a
ember-bootstrap configuration:
bootstrapVersion: 5
importBootstrapFont: false
importBootstrapCSS: false

[jelhan]

Is it working if using @direction="start" and @direction="end"? I also recently wondered if left and right values are still correct for those arguments if using BS5 when reviewing the Glint types (#1861). Maybe it is just an error in API docs?

[silhusk]

By using @direction="start" it does add the correct class dropstart , but then popper is given a wrong argument and the menu opens down (the default).

ember-bootstrap/addon/components/bs-dropdown/menu.js

Line 129 in 4ec513d

} else if (direction === 'left') {

[jelhan]

Thanks for investigating! I wasn't aware that it's also used with Popper. Do you know how that misalignment on namings is solved in Bootstrap itself?

[silhusk]

Relevant snippets taken from https://github.com/twbs/bootstrap/blob/cbc4e3a4098506315c32346b194e9e1a4d7db091/js/src/dropdown.js

// Line 50
const CLASS_NAME_DROPEND = 'dropend'
const PLACEMENT_RIGHT = isRTL() ? 'left-start' : 'right-start'

// Line 248
_getPlacement() {
  const parentDropdown = this._parent

  if (parentDropdown.classList.contains(CLASS_NAME_DROPEND)) {
    return PLACEMENT_RIGHT
  }

  if (parentDropdown.classList.contains(CLASS_NAME_DROPSTART)) {
    return PLACEMENT_LEFT
  }
  ...
}

// Line 295
_getPopperConfig() {
  const defaultBsPopperConfig = {
    placement: this._getPlacement(),
    ...
  }
  ...
}

[jelhan]

Thanks a lot for looking up that information!

I feel we should do the following:

1. Fix @direction="left and @direction="right" to apply correct classes for Bootstrap 5 (dropstart, dropend). That would fix the immediate bug.
2. Rename left and right to start and end not only in this case but for all arguments. We may want to keep the old names as deprecated alias for now to ease migration.

PR for the short-term fix listed as first step is very welcome. Do you have time working on it?

I would like giving @simonihmig the chance to raid concerns with second step, the remaining of argument values, before picking it up.

[simonihmig]

Yeah, I wouldn't mind doing 2., if anyone has the time for that!? 🙃