No scrollbar in Select component when menu has dividers

[evansjohnson]

Fixes #6780

Checklist

• Includes tests
• Update documentation

I don't think these are relevant for this PR

Changes proposed in this pull request:

For a menu divider inside of a select-popover, we already have applied a 5px padding inside the popover. This means to get to full popover width we need to apply the -5px margin to the outer list element, and therefor also undo the -5px margin on the menu divider.

This PR is a variation on #6786, which correctly identified that the -5px margin on the menu divider is responsible for the scrollbar being added, but this PR additionally applies this -5px margin on the list element so that the divider continues to take up the full width.

Reviewers should focus on:

• Any other styling options I'm not thinking of that may be negatively impacted
• Any concerns of this fix itself being a breaking change, since this style issue has likely existed since [core] feat: add MenuItem small modifier, extend MenuDivider width #6269 was merged almost a year ago?

Screenshot

Before

After

*note right and bottom padding outside of list are now also removed. this seems to match what we do in the menu popover

[changelog-app]

Generate changelog in packages/core/changelog/@unreleased

What do the change types mean?

• feature: A new feature of the service.
• improvement: An incremental improvement in the functionality or operation of the service.
• fix: Remedies the incorrect behaviour of a component of the service in a backwards-compatible way.
• break: Has the potential to break consumers of this service's API, inclusive of both Palantir services
  and external consumers of the service's API (e.g. customer-written software or integrations).
• deprecation: Advertises the intention to remove service functionality without any change to the
  operation of the service itself.
• manualTask: Requires the possibility of manual intervention (running a script, eyeballing configuration,
  performing database surgery, ...) at the time of upgrade for it to succeed.
• migration: A fully automatic upgrade migration task with no engineer input required.

Note: only one type should be chosen.

How are new versions calculated?

• ❗The break and manual task changelog types will result in a major release!
• 🐛 The fix changelog type will result in a minor release in most cases, and a patch release version for patch branches. This behaviour is configurable in autorelease.
• ✨ All others will result in a minor version release.

Type

• Feature
• Improvement
• Fix
• Break
• Deprecation
• Manual task
• Migration

Description

No scrollbar in Select component when menu has dividers

---

Generate changelog in packages/select/changelog/@unreleased

What do the change types mean?

• feature: A new feature of the service.
• improvement: An incremental improvement in the functionality or operation of the service.
• fix: Remedies the incorrect behaviour of a component of the service in a backwards-compatible way.
• break: Has the potential to break consumers of this service's API, inclusive of both Palantir services
  and external consumers of the service's API (e.g. customer-written software or integrations).
• deprecation: Advertises the intention to remove service functionality without any change to the
  operation of the service itself.
• manualTask: Requires the possibility of manual intervention (running a script, eyeballing configuration,
  performing database surgery, ...) at the time of upgrade for it to succeed.
• migration: A fully automatic upgrade migration task with no engineer input required.

Note: only one type should be chosen.

How are new versions calculated?

• ❗The break and manual task changelog types will result in a major release!
• 🐛 The fix changelog type will result in a minor release in most cases, and a patch release version for patch branches. This behaviour is configurable in autorelease.
• ✨ All others will result in a minor version release.

Type

• Feature
• Improvement
• Fix
• Break
• Deprecation
• Manual task
• Migration

Description

No scrollbar in Select component when menu has dividers

---

Check the box to generate changelog(s)

• Generate changelog entry

[svc-palantir-github]

no select scrollbar with dividers

Build artifact links for this commit: documentation | landing | table | demo

This is an automated comment from the deploy-preview CircleCI job.

[svc-palantir-github]

alpha order

Build artifact links for this commit: documentation | landing | table | demo

This is an automated comment from the deploy-preview CircleCI job.

[chaejunlee]

I think going over the <Menu /> is necessary. Can you post the screenshot of it?

[evansjohnson]

There's no changes to the menu in this PR, except when inside a select popover

I checked for good measure and in the generated docs preview you can see the styling menu is unaffected:

Note in #6786 since the -5px margin is removed there is a slight visual regression in the screenshot where the border no longer goes to the edge of the menu, which seems to be what the intent of that style is.

[gluxon]

Menu Items Flush

Is it intended that the menu items themselves are flush with the border of the popover after these changes? I see from your description that we wanted to make the horizontal dividers flush, which I agree with.

Here's the before picture again for reference. I think the intended visual design was to have a bit of spacing around them menu item since the light blue hover background has a slight border radius. That border radius doesn't look as great if the background is how flush with the border.

Suggestion

I'm wondering if we should tweak @mixin menu-divider()'s implementation to instead be a ::before pseudo-element that is "out of flow".

The negative margin would then only be on the pseudo-element driving the horizontal border. Since the pseudo-element is "out of flow", we would eliminate unintended CSS layout calculations (like this one) entirely in the future.