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

feat(select): ghost select #1133

Merged
merged 9 commits into from Dec 14, 2021
Merged

feat(select): ghost select #1133

merged 9 commits into from Dec 14, 2021

Conversation

rachelbt
Copy link
Contributor

@rachelbt rachelbt commented Nov 22, 2021

Quick and dirty adding a ghost layout.
Unable to use our layout because there's already a Layout function in MVC.
Ghost select only goes with dense. As Joella asked.

image

@rachelbt rachelbt added the Work in Progress You're welcome to have a look around and drop your early comments label Nov 22, 2021
@github-actions
Copy link

🚀

Latest successful build of the PR deployed here.

🚀

@rachelbt rachelbt removed the Work in Progress You're welcome to have a look around and drop your early comments label Nov 25, 2021
@@ -48,6 +48,9 @@ export class VWCSelect extends MWCSelect {
@property({ type: String, reflect: true })
name: string | undefined;

@property({ type: Boolean, reflect: true, attribute: 'ghost' })
Copy link
Contributor

Choose a reason for hiding this comment

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

This needs to be tested.
Also, need to test the logic that's bound to it:
If ghost is true, dense must be true (at least that's what I see in the code).

Copy link
Contributor

Choose a reason for hiding this comment

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

should we enforce programmatically? it's that mutating user's definition discussion again...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We need because of the design. As I wrote in the summery of this PR its a quick & dirty fix. This component is different from our conventions in a lot of ways...

@yinonov yinonov marked this pull request as draft December 8, 2021 13:58
@yinonov yinonov marked this pull request as ready for review December 8, 2021 13:58
Copy link
Contributor

@YonatanKra YonatanKra left a comment

Choose a reason for hiding this comment

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

LGTM

@sonarcloud
Copy link

sonarcloud bot commented Dec 13, 2021

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@rachelbt rachelbt merged commit 93426d1 into master Dec 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants