feat(b-nav-item-dropdown): improve default handling of dropdown toggl…

…e link (closes #3942) (#5344)

* fix(b-nav-item-dropdown): let `<b-link>` handle `href` default

* Update nav-item-dropdown.js

* Update nav-item-dropdown.spec.js

* Update nav-item-dropdown.js

* Update nav-item-dropdown.js

* Update id.js

* Update nav-item-dropdown.js

* Update nav-item-dropdown.spec.js

* Update nav-item-dropdown.spec.js

* Update README.md

* Update nav-item-dropdown.spec.js

* Update nav-item-dropdown.js

* Update nav-item-dropdown.spec.js

* Update nav-item-dropdown.spec.js

* Update nav-item-dropdown.spec.js

* Update README.md

* Update nav-item-dropdown.js

* Update nav-item-dropdown.spec.js

Co-authored-by: Troy Morehouse <troymore@nbnet.nb.ca>

src/components/nav/README.md

@@ -190,12 +190,13 @@ Use `<b-nav-item-dropdown>` to place dropdown items within your nav.
 
 Sometimes you want to add your own class names to the generated dropdown toggle button, that by
 default have the classes `nav-link` and `dropdown-toggle`. Use the `toggle-class` prop to add them
-(like above) which will produce something like:
+(like above) which will render HTML similar to:
 
 ```html
 <li id="my-nav-dropdown" class="nav-item b-nav-dropdown dropdown">
   <a
-    href="#"
+    role="button"
+    href="#my-nav-dropdown"
     id="my-nav-dropdown__BV_button_"
     aria-haspopup="true"
     aria-expanded="false"
@@ -222,6 +223,17 @@ shown. When there are a large number of dropdowns rendered on the same page, per
 impacted due to larger overall memory utilization. You can instruct `<b-nav-item-dropdown>` to
 render the menu contents only when it is shown by setting the `lazy` prop to true.
 
+### Dropdown implementation note
+
+Note that the toggle button is actually rendered as a link `<a>` tag with `role="button"` for
+styling purposes, and typically has the `href` set to `#` unless an ID is provided via the `id`
+prop.
+
+The toggle will prevent scroll-top-top behaviour (via JavaScript) when clicking the toggle link. In
+some cases when using SSR, and the user clicks the toggle button _before_ Vue has had a chance to
+hydrate the component, the page will scroll to top. In these cases, simply providing a unique ID via
+the `id` prop will prevent the unwanted scroll-to-top behaviour.
+
 ## Nav text content
 
 Use the `<b-nav-text>` child component to place plain text content into the nav:

src/components/nav/nav-item-dropdown.js

@@ -1,25 +1,28 @@
 import Vue from '../../utils/vue'
-import { props as BDropdownProps } from '../dropdown/dropdown'
-import idMixin from '../../mixins/id'
-import dropdownMixin from '../../mixins/dropdown'
-import normalizeSlotMixin from '../../mixins/normalize-slot'
 import pluckProps from '../../utils/pluck-props'
 import { htmlOrText } from '../../utils/html'
+import dropdownMixin from '../../mixins/dropdown'
+import idMixin from '../../mixins/id'
+import normalizeSlotMixin from '../../mixins/normalize-slot'
+import { props as BDropdownProps } from '../dropdown/dropdown'
 import { BLink } from '../link/link'
 
-// -- Constants --
-
+// --- Props ---
 export const props = pluckProps(
   ['text', 'html', 'menuClass', 'toggleClass', 'noCaret', 'role', 'lazy'],
   BDropdownProps
 )
 
+// --- Main component ---
 // @vue/component
 export const BNavItemDropdown = /*#__PURE__*/ Vue.extend({
   name: 'BNavItemDropdown',
   mixins: [idMixin, dropdownMixin, normalizeSlotMixin],
   props,
   computed: {
+    toggleId() {
+      return this.safeId('_BV_toggle_')
+    },
     isNav() {
       // Signal to dropdown mixin that we are in a navbar
       return true
@@ -41,57 +44,62 @@ export const BNavItemDropdown = /*#__PURE__*/ Vue.extend({
     }
   },
   render(h) {
-    const button = h(
+    const { toggleId, visible } = this
+
+    const $toggle = h(
       BLink,
       {
-        ref: 'toggle',
         staticClass: 'nav-link dropdown-toggle',
         class: this.toggleClasses,
         props: {
-          href: '#',
+          href: `#${this.id || ''}`,
           disabled: this.disabled
         },
         attrs: {
-          id: this.safeId('_BV_button_'),
+          id: toggleId,
+          role: 'button',
           'aria-haspopup': 'true',
-          'aria-expanded': this.visible ? 'true' : 'false'
+          'aria-expanded': visible ? 'true' : 'false'
         },
         on: {
           mousedown: this.onMousedown,
           click: this.toggle,
           keydown: this.toggle // Handle ENTER, SPACE and DOWN
-        }
+        },
+        ref: 'toggle'
       },
       [
-        this.$slots['button-content'] ||
-          this.$slots.text ||
+        // TODO: The `text` slot is deprecated in favor of the `button-content` slot
+        this.normalizeSlot(['button-content', 'text']) ||
           h('span', { domProps: htmlOrText(this.html, this.text) })
       ]
     )
-    const menu = h(
+
+    const $menu = h(
       'ul',
       {
         staticClass: 'dropdown-menu',
         class: this.menuClasses,
-        ref: 'menu',
         attrs: {
           tabindex: '-1',
-          'aria-labelledby': this.safeId('_BV_button_')
+          'aria-labelledby': toggleId
         },
         on: {
           keydown: this.onKeydown // Handle UP, DOWN and ESC
-        }
+        },
+        ref: 'menu'
       },
-      !this.lazy || this.visible ? this.normalizeSlot('default', { hide: this.hide }) : [h()]
+      !this.lazy || visible ? this.normalizeSlot('default', { hide: this.hide }) : [h()]
     )
+
     return h(
       'li',
       {
         staticClass: 'nav-item b-nav-dropdown dropdown',
         class: this.dropdownClasses,
         attrs: { id: this.safeId() }
       },
-      [button, menu]
+      [$toggle, $menu]
     )
   }
 })