Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added borderless attr for power select #479

Merged
merged 6 commits into from
Mar 12, 2025
Merged

Added borderless attr for power select #479

merged 6 commits into from
Mar 12, 2025

Conversation

OwenCoogan
Copy link
Contributor

What does this PR do?

Capture d’écran 2025-03-10 à 11 10 22

Related to: #

What are the observable changes?

Good PR checklist

  • Title makes sense
  • Is against the correct branch
  • Only addresses one issue
  • Properly assigned
  • Added/updated tests
  • Added/updated documentation
  • Migrated touched components to Glimmer Components
  • Properly labeled

@OwenCoogan OwenCoogan self-assigned this Mar 10, 2025
Copy link

linear bot commented Mar 10, 2025

@OwenCoogan OwenCoogan marked this pull request as ready for review March 10, 2025 10:11
@@ -1,5 +1,5 @@
<div
class="upf-power-select fx-1 fx-col"
class="upf-power-select fx-1 fx-col {{if @borderless 'borderless'}}"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
class="upf-power-select fx-1 fx-col {{if @borderless 'borderless'}}"
class="upf-power-select fx-1 fx-col {{if @borderless 'upf-power-select--borderless'}}"

To better respect the BEM, maybe more upf-power-select--borderless

@@ -16,6 +16,12 @@
}
}

&.borderless {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
&.borderless {
&--borderless {

</OSS::PowerSelect>
`);

assert.dom('.upf-power-select').hasClass('borderless');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
assert.dom('.upf-power-select').hasClass('borderless');
assert.dom('.upf-power-select').hasClass('upf-power-select--borderless');

@@ -40,6 +40,16 @@ export default {
},
control: { type: 'boolean' }
},
borderless: {
description: 'Display border of the component',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Hide the border around the dropdown component"

Copy link
Contributor

@JulienVannier66 JulienVannier66 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@OwenCoogan OwenCoogan merged commit 8b3b46f into master Mar 12, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants