microsoft / fluentui

Fluent UI web represents a collection of utilities, React components, and web components for building web applications.
https://react.fluentui.dev
Other
18.44k stars 2.72k forks source link

[Bug]: 8.64.2 regresses padding-top styling for Panels with custom header/nav #22476

Closed dbjorge closed 2 years ago

dbjorge commented 2 years ago

Library

React / v8 (@fluentui/react)

System Info

System:
    OS: Windows 10 10.0.22000
    CPU: (16) x64 Intel(R) Xeon(R) W-11955M CPU @ 2.60GHz
    Memory: 90.52 GB / 127.67 GB
  Browsers:
    Edge: Spartan (44.22000.120.0), Chromium (100.0.1185.39)
    Internet Explorer: 11.0.22000.120
  npmPackages:
    @fluentui/react: ^8.64.2 => 8.64.2

Are you reporting Accessibility issue?

no

Reproduction

https://codepen.io/dbjorge/pen/yLpxJXb

Bug Description

I believe this is a regression introduced by https://github.com/microsoft/fluentui/pull/22408. It changes the implementation of the empty space in question from marginTop to paddingTop, but appears to have missed adjusting the corresponding behavior to suppress the empty space when custom navigation is in play at L271 of Panel.styles.ts - it is still overriding marginTop in that case, rather than the newly-used paddingTop.

Actual Behavior

As of @fluentui/react@8.64.2, there is a new 18px of empty padding applied above custom header+nav rendering passed to <Panel>s. See red-highlighted section of the below screenshot of the provided minimal repro:

screenshot in 8.64.2 demonstrating issue

Expected Behavior

Behavior should be consistent with @fluentui/react<=8.64.1, where the default 18px of margin-top applied to <Panel> content was suppressed for <Panel>s which use a custom header/nav. See red pointer of the below screenshot of the same repro downgraded to 8.64.1:

screenshot in 8.64.1 demonstrating expected behavior

Logs

No response

Requested priority

High

Products/sites affected

Accessibility Insights for Web (see https://github.com/microsoft/accessibility-insights-web/pull/5312#issuecomment-1097091341 for more details and a screenshot of the motivating example)

Are you willing to submit a PR to fix?

no

Validations

micahgodbolt commented 2 years ago

@Hotell can you look into the custom header/nav scenario with the fix you made last week? We might need to take a different approach on #22408