- Notifications
You must be signed in to change notification settings - Fork352
Onboarding components fixes#1393
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
Uh oh!
There was an error while loading.Please reload this page.
Changes fromall commits
d77a08556f8d5b288b930119c20e52f6f588e0edddab123451e58f5f1356a7a7c33e42d88f3d5c2e4efcf8b614439f901cb7c6cad6d1b89ec81672aba0ecabd7adb48e1f4fc3e569e9fc26fde7821c3936b353164879161abf06fe8ea6cefFile filter
Filter by extension
Conversations
Uh oh!
There was an error while loading.Please reload this page.
Jump to
Uh oh!
There was an error while loading.Please reload this page.
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more abouthow customized files appear on GitHub.
Uh oh!
There was an error while loading.Please reload this page.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| use pgml_components::{component,Component}; | ||
| use sailfish::TemplateOnce; | ||
| #[derive(TemplateOnce,Default)] | ||
| #[template(path ="cards/primary/template.html")] | ||
| pubstructPrimary{ | ||
| component:Component, | ||
| style:String, | ||
| } | ||
| implPrimary{ | ||
| pubfnnew(component:Component) ->Primary{ | ||
| Primary{ | ||
| component, | ||
| style:"".into(), | ||
| } | ||
| } | ||
| pubfnz_index(mutself,index:i64) ->Self{ | ||
| self.style =format!("position: relative; z-index: {};", index); | ||
| self | ||
| } | ||
Comment on lines +19 to +22 Contributor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. Same thing as the next comment, is it possible to solve this with a wrapper? ContributorAuthor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. Same issue as above. The wrapper would have to modify the style of its child. | ||
| } | ||
| component!(Primary); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| div[data-controller="cards-primary"] { | ||
| border-radius:#{$card-border-radius}; | ||
| padding:#{$card-spacer-y}#{$card-spacer-x}; | ||
| box-shadow:#{$card-box-shadow}; | ||
| background-color:#{$gray-800}; | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| <div data-controller="cards-primary" style="<%- style %>"> | ||
| <%+ component %> | ||
| </div> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,12 +1,18 @@ | ||
| use pgml_components::{component, Component}; | ||
| use sailfish::TemplateOnce; | ||
| use crate::components::stimulus::StimulusAction; | ||
| use crate::types::CustomOption; | ||
| #[derive(TemplateOnce)] | ||
| #[template(path = "cards/rgb/template.html")] | ||
| pub struct Rgb { | ||
| value: Component, | ||
| link: Option<String>, | ||
| link_action: CustomOption<StimulusAction>, | ||
| controller_classes: Vec<String>, | ||
| card_classes: Vec<String>, | ||
| body_classes: Vec<String>, | ||
| } | ||
| impl Default for Rgb { | ||
| @@ -19,20 +25,44 @@ impl Rgb { | ||
| pub fn new(value: Component) -> Rgb { | ||
| Rgb { | ||
| value, | ||
| link: None, | ||
| link_action: CustomOption::default(), | ||
| controller_classes: vec![], | ||
| card_classes: vec![], | ||
| body_classes: vec![], | ||
| } | ||
| } | ||
| pub fn active(mut self) -> Self { | ||
| self.card_classes.push("active".into()); | ||
| self.card_classes.push("main-gradient-border-card-1".into()); | ||
| self | ||
| } | ||
| pub fn is_active(mut self, active: bool) -> Self { | ||
| if active { | ||
| self.card_classes.push("active".into()); | ||
| self.card_classes.push("main-gradient-border-card-1".into()); | ||
| } | ||
| self | ||
| } | ||
| pub fn link(mut self, link: &str) -> Self { | ||
| self.link = Some(link.to_string()); | ||
| self | ||
| } | ||
| pub fn link_action(mut self, action: StimulusAction) -> Self { | ||
| self.link_action = action.into(); | ||
| self | ||
| } | ||
| pub fn h_100(mut self) -> Self { | ||
| self.controller_classes.push("h-100".into()); | ||
| self.card_classes.push("h-100".into()); | ||
| self | ||
Comment on lines +61 to +64 Contributor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. for something like this is it possible to just always have the h-100 on and for instances where you need to limit the height, place it in a wrapper that does not have h-100? Just brainstorming, this could turn into a slippery slope of having a function for every specific situation. I think if it is possible to solve problems like this with wrappers, we should. ContributorAuthor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. By default, the card occupies as much space as it needs to render its contents. The A wrapper can't make its child occupy 100% of its space unless we add additional CSS to the wrapper knowing that the child needs to do this. For example, here we could do something like: .card-wrapper-h-100 { @extend.h-100.card { @extend.h-100.card-body { @extend.h-100 } }} which now that I'm writing this isn't too bad. One issue with this is more of a theoretical technicality, where a component "reaches inside" another component and modifies its state which could cause weird bugs later on. It's typically best to have the component use public methods to modify its state in a predicable way. Maybe if we name this slightly differently, it could be better? I'm not leaning strongly one way or the other. | ||
| } | ||
| } | ||
| component!(Rgb); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| use pgml_components::{component, Component}; | ||
| use sailfish::TemplateOnce; | ||
| #[derive(TemplateOnce, Default)] | ||
| #[template(path = "cards/secondary/template.html")] | ||
| pub struct Secondary { | ||
| value: Component, | ||
| } | ||
| impl Secondary { | ||
| pub fn new(value: Component) -> Secondary { | ||
| Secondary { value } | ||
| } | ||
| } | ||
| component!(Secondary); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| div[data-controller="cards-secondary"] { | ||
| .card { | ||
| --bs-card-bg:transparent; | ||
| --bs-card-border-color:#{$neon-tint-100}; | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| <divdata-controller="cards-secondary"> | ||
| <divclass="card"> | ||
| <divclass="card-body"> | ||
| <%+ value %> | ||
| </div> | ||
| </div> | ||
| </div> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| span[data-controller="headings-gray"] { | ||
| color:#{$gray-400}; | ||
| } | ||
Comment on lines +1 to +3 Contributor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. You could add color = gray-400 to the typography.scss file and just use the class here. Utility classes for the gray scale is always nice. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| use pgml_components::component; | ||
| use sailfish::TemplateOnce; | ||
| #[derive(TemplateOnce,Default)] | ||
| #[template(path ="headings/gray/template.html")] | ||
| pubstructGray{ | ||
| value:String, | ||
| } | ||
| implGray{ | ||
Contributor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. If we want to go down this road, maybe one heading that takes a color, or has a setter function for the desired color, rather than a new component for every header. ContributorAuthor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. Yeah I agree, these are too small to be individual components. | ||
| pubfnnew(value:implToString) ->Gray{ | ||
| Gray{ | ||
| value: value.to_string(), | ||
| } | ||
| } | ||
| } | ||
| component!(Gray); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| <span | ||
| data-controller="headings-gray"> | ||
| <%= value %> | ||
| </span> |
Uh oh!
There was an error while loading.Please reload this page.