patternfly / patternfly-elements

PatternFly Elements. A set of community-created web components based on PatternFly design.
https://patternflyelements.org/
MIT License
377 stars 92 forks source link

fix(select): hr styles #2701

Closed bennypowers closed 6 months ago

bennypowers commented 7 months ago

Closes #2700

What I did

  1. implement styles for pf select hr separators

Testing Instructions

  1. compare to http://v4-archive.patternfly.org/v4/components/select/
changeset-bot[bot] commented 7 months ago

⚠️ No Changeset found

Latest commit: 0ef98b8453dc80fe72a799159e24c430b5e97892

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

github-actions[bot] commented 7 months ago

Deploy Preview for patternfly-elements ready!

Name Link
Latest commit 6721eaccf24e1de1753deacdc3dfa4fa35d9824f
Deploy Preview https://deploy-preview-2701--patternfly-elements.netlify.app/

To edit notification comments on pull requests, go to your Netlify site settings.

netlify[bot] commented 7 months ago

Deploy Preview for patternfly-elements ready!

Name Link
Latest commit 0ef98b8453dc80fe72a799159e24c430b5e97892
Latest deploy log https://app.netlify.com/sites/patternfly-elements/deploys/65ef039612e90200084f3bc3
Deploy Preview https://deploy-preview-2701--patternfly-elements.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

bennypowers commented 7 months ago

pf-select-group has a border bottom creating the illusion of a double border when the hr appears after it

Can we just consider that an antipattern, then?

zeroedin commented 6 months ago

pf-select-group has a border bottom creating the illusion of a double border when the hr appears after it

Can we just consider that an antipattern, then?

So if I understand correctly, <hr> is slotted with <pf-option> but when an <pf-option-group> is used you don't need to add the <hr> manually.

I feel like the use case is different enough to have a different expectation on the implementation. pf-option-group is an explicit pattern and grouping, where as the <hr> between <pf-option> is an additive visual reference of a perceived separation, but technically all the options are still the same "group".

bennypowers commented 6 months ago

@nikkimk don't forget the gh approval, plznthx