NEXT-35455 - fix dropdown account menu still showing if the offcanvas…

[divide29]

1. Why is this change necessary?

• Currently the dropdown account menu is still being displayed, even if the offcanvas variant is present.

2. What does this change do, exactly?

• Changed hiddenClass to showClass
• Changed d-none to show
• switched classList add/remove for this._dropdown

3. Describe each step to reproduce the issue or behaviour.

• Switch to viewport XS/ SM
• Open the account menu

4. Please link to the relevant issues (if any).

• https://issues.shopware.com/issues/NEXT-35455

5. Checklist

• I have rebased my changes to remove merge conflicts
• I have written tests and verified that they fail without my change
• I have created a changelog file with all necessary information about my changes
• I have written or adjusted the documentation according to my changes
• This change has comments for package types, values, functions, and non-obvious lines of code
• I have read the contribution requirements and fulfil them.

[CLAassistant]

All committers have signed the CLA.

[marcelbrode]

@tobiasberge Do you think this is considered a break?
If not, I'd approve this MR 🙂

[tobiasberge]

Strictly speaking this would be a break, because the plugin option hiddenClass is now gone and has no effect anymore if used e.g. to pass a custom CSS class.

We could leave the "old" hiddenClass option as well as the classList assignment alongside with the new showClass. Then there would be some "dead" code but we avoid the breaking change:

/** @deprecated tag:v6.7.0 - Use "showClass" instead. */
hiddenClass: 'd-none',

showClass: 'show',

/*
 ----
*/

/** @deprecated tag:v6.7.0 - "hiddenClass" is deprecated. "showClass" will be used instead. */
this._dropdown.classList.remove(this.options.hiddenClass);
this._dropdown.classList.add(this.options.showClass);

---

Coincidentally I was also working on this ticket. 😆

I had a different approach and tried to omit the opening of the dropdown completely when in mobile viewport. Right now the Bootstrap dropdown still opens in the background, and is then attempted to be hidden again via "d-none" class manually.

Only thing that was in progress with my approach was the "toggling" in _onViewportHasChanged. I also deprecated the "hiddenClass" but without replacement with another "hide/show" class. Instead everything should happen via Bootstraps dropdown API directly without manual CSS classes.

Normally, the "show" class is set by Bootstrap itself "internally". But if this proposed solution here gets the job down, I'm also fine with it. 🙂 What do you think? @marcelbrode @divide29

This was roughly my approach:

    _registerEventListeners() {
        this.el.addEventListener('click', this._onClickAccountMenuTrigger.bind(this, this.el));
+        this._dropdownWrapper.addEventListener('show.bs.dropdown', this._onClickPreventDropdown.bind(this));

    /**
     * Prevent opening the Bootstrap dropdown in allowed viewports
     *
     * @param event
     * @private
     */
    _onClickPreventDropdown(event) {
        if (this._isInAllowedViewports() === true) {
            event.preventDefault();
        }
    }

[divide29]

Yeah, i think your approach to prevent the event is the better one and i'm completly fine with it, thank you! :)

[marcelbrode]

I also with your solution, Tobi :)

Since @divide29 is fine with that, shall we close this?

[tobiasberge]

Thank you for your feedback. 👍 We will leave this PR open until the other solution is finished. If it doesn't work out we can still use your proposal @divide29.