blueprint
blueprint copied to clipboard
No scrollbar in Select component when menu has dividers
Fixes https://github.com/palantir/blueprint/issues/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 https://github.com/palantir/blueprint/pull/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 https://github.com/palantir/blueprint/pull/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
Generate changelog in packages/core/changelog/@unreleased
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
andmanual 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
Generate changelog in packages/select/changelog/@unreleased
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
andmanual 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
Check the box to generate changelog(s)
- [ ] Generate changelog entry
no select scrollbar with dividers
Build artifact links for this commit: documentation | landing | table | demoThis is an automated comment from the deploy-preview CircleCI job.
alpha order
Build artifact links for this commit: documentation | landing | table | demoThis is an automated comment from the deploy-preview CircleCI job.
I think going over the <Menu />
is necessary. Can you post the screenshot of it?
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 https://github.com/palantir/blueprint/pull/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.
Thanks for the close look and good catch @gluxon!
I wrapped up this approach since it was easy to add back the margin one more time for the items, but given how much back and forth this is I may try the psuedo element approach quickly as well.
As a nice to have - it looks like there was a small stying weirdness where there is empty space padding to the left but not right of the selection. With fixing the scrollbar overflow this also sizes the menu items correctly to have padding on both sides.
develop
this PR
with scrollbars showing only when scrolling still looks good
add side padding around selection highlight
Build artifact links for this commit: documentation | landing | table | demoThis is an automated comment from the deploy-preview CircleCI job.
better approach?
Build artifact links for this commit: documentation | landing | table | demoThis is an automated comment from the deploy-preview CircleCI job.
As a nice to have - it looks like there was a small stying weirdness where there is empty space padding to the left but not right of the selection. With fixing the scrollbar overflow this also sizes the menu items correctly to have padding on both sides.
Very nice! 😁
Update packages/select/src/components/select/_select.scss
Build artifact links for this commit: documentation | landing | table | demoThis is an automated comment from the deploy-preview CircleCI job.