- Notifications
You must be signed in to change notification settings - Fork328
onboarding modules#1391
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
onboarding modules#1391
Uh oh!
There was an error while loading.Please reload this page.
Conversation
| span.material-symbols-outlined { | ||
| color: #{$white}; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
You can also just add text-white to the class of this component.
| span[data-controller="badges-large-label"] { | ||
| padding: 8px; | ||
| background: #{$gray-500}; | ||
| border-radius: 8px; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
bs-border4-radius or class rounded-2
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
--bs-border-radius sets the border radius for Bootstrap components that declare theborder-radius: var(--bs-border-radius) in their CSS. This is a new component, so we have to set the border radius with probablyrounded-2 as you suggested.
| @@ -0,0 +1,6 @@ | |||
| div[data-controller="cards-rgb"] { | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I can't tell but If this doesn't have a hover state, it needs one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
You're right, it doesn't have one. I don't think there is one in the design. I'll flag it for review and follow up with a PR.
| @@ -0,0 +1,18 @@ | |||
| use pgml_components::component; | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Can we just use our text-gradient-blue class and remove this component?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Yes we can. I think though CSS classes are harder to discover than components. I'm starting to think we should lean on components even more to make our UI more consistent and re-usable. I can see in the future our HTML templates being composed entirely of code like<%+ SomeComponent::new() %>.
| @@ -0,0 +1,18 @@ | |||
| use pgml_components::component; | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Same as above, can we just just text-gradient-green class here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I should definitely re-use that class here.
| export default class extends Controller { | ||
| static targets = ["button"]; | ||
| onClick(e) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Change this name as recommended byhttps://stimulus.hotwired.dev/reference/actions#naming-conventions
| div[data-controller="inputs-text-search"] { | ||
| .dropdown { | ||
| .dropdown-menu { | ||
| padding: 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
why not just add p-0 class to html?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Because this re-uses theDropdown component which doesn't support (and shouldn't in my opinion) adding custom classes to an element. This overrides theDropdown style.
| @@ -0,0 +1,19 @@ | |||
| import { Controller } from "@hotwired/stimulus"; | |||
| export default class extends Controller { | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Do you want something to limit the query speed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Yeah, do we have anything? I think we have underscore and_.debounce?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Yeah I believe it comes from lodash.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Ah yeah, lodash is the new library replacing underscore. I'm old.
| @@ -0,0 +1,160 @@ | |||
| <% use crate::components::tables::small::*; %> | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
No need to have playground and demo.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Ah yeah, I was looking for playground and didn't find it. That's weird...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Playground was made so long ago it is still hidden in the templates and is not a component. It should probably be refactored into a component. Maybe we move the playground stuff to your demo page an remove playground.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Works for me. It's just a place to make sure components work before using them in our other apps.
| @@ -120,6 +120,7 @@ $hp-white: #{$gray-200}; | |||
| // Background Colors | |||
| $bg-white: #F8FAFC; | |||
| $bg-black: #0D0D0E; | |||
| $bg-black-1: #17181A; | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
We shouldn't need to add any colors. this is gray-800. If we need this variable to it like$bg-black-1: #{$gray-800}; or just use gray-800 in the css you are trying to use this for.
Going to probably have to make more changes as I integrate this into our main app. More PRs to follow. |
Uh oh!
There was an error while loading.Please reload this page.
New components
Small table
Green heading
Blue heading
Input with optional icon
Dropdown input with search
Dropdown input with backend-powered search.
Badges (small)
Badges (large)
Range input v2
Switch v2
Radio
Select (dropdown) with display/value separation
Select supports different values for display in the option and the
<input>field. This is useful when display options aren't easily serializable to values the backend can understand.RGB card
Fixes
<ul>(now it's 20px top and 20px bottom padding)Debugimplementation forComponent