soflyy / breakdance-bugs

Bug reports from Breakdance users.
40 stars 6 forks source link

getCurrentBreakpoint() in breakdance-utils.js returns wrong breakpoint #937

Closed MrBushid0 closed 9 months ago

MrBushid0 commented 1 year ago

Hi,

The function getCurrentBreakpoint() in the frontend utils returns the wrong breakpoint which breaks the functionality of anything that relies on it.

Fix:

In breakdance-utils.js > getCurrentBreakpoint function

Change the includes() parameter to test for the object instead of the id prop

const activeBreakpoint = breakpoints .filter((b) => { if (!availableBreakpoints) return true; return availableBreakpoints.includes(b.id); }) .sort((a, b) => a.maxWidth - b.maxWidth) .find((b) => matchMedia(b));

To

const activeBreakpoint = breakpoints .filter((b) => { if (!availableBreakpoints) return true; return availableBreakpoints.includes(b); }) .sort((a, b) => a.maxWidth - b.maxWidth) .find((b) => matchMedia(b));

Thanks!

felipemarcos commented 9 months ago

@MrBushid0 This function actually accepts an array of IDs, we use it internally in some elements and we haven't had any issues so far.

I went ahead and improved the comments above the function and renamed the param so its intent is clear. This change should land the next major release or so, but here is a preview:

  /**
   * Return the current active breakpoint, if none is active returns the Desktop breakpoint, which is not a real breakpoint.
   * @param {string[]|undefined} breakpointIds List of breakpoints to check.
   * @returns {Breakpoint}
   */
  function getCurrentBreakpoint(breakpointIds) {
    const { BASE_BREAKPOINT_ID, breakpoints } = window.BreakdanceFrontend.data;

    const activeBreakpoint = breakpoints
      .filter((b) => {
        if (!breakpointIds) return true;
        return breakpointIds.includes(b.id);
      })
      .sort((a, b) => a.maxWidth - b.maxWidth)
      .find((b) => matchMedia(b));

    return (
      activeBreakpoint || breakpoints.find((b) => b.id === BASE_BREAKPOINT_ID)
    );
  }

Example

const { getCurrentBreakpoint } = BreakdanceFrontend.utils;

getCurrentBreakpoint(['breakpoint_phone_portrait', 'breakpoint_phone_landscape'])
MrBushid0 commented 9 months ago

@felipemarcos I see. Thanks for the clarification

Indeed It's working as intended in that case. In my scenario, I was using the list of active breakpoints ( objects not ids ).

Thanks