styling updates #50
styling updates #50
Conversation
michellexliu
commented
Aug 2, 2021
michellexliu
commented
- Added _ to .scss file names
- Re-factored styling_classnames file into @extend in sass
- Updated spacing (decreased padding between buttons by 1 token, fixed focus state margin issues, decreased margins between checkboxes by 1 token, smaller line spacing for title, more space between header & body text)
- Added drop shadow on module
… module, fixed focus state margin issue
…et innerhtml to p in order to control spacing in between paragraphs
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://salmon-moss-01b532c0f-50.eastus2.azurestaticapps.net |
| @@ -65,15 +64,15 @@ export const TRANSLATIONS_EN = { | |||
| title: | |||
| "Thank you for your feedback! It has been submitted.<br /><br />Your feedback is anonymous and confidential, so you will not receive a reply.", | |||
| plainText: | |||
| "The City of New York is always trying to improve its services. Are you interesed in being a user research participant? This is is entirely voluntary and you can opt out at any time. Signing up to be a user research participant will have no impact on your feedback today, or your eligibility to access or receive services in the future.", | |||
| "<p>The City of New York is always trying to improve its services. Are you interesed in being a user research participant? This is is entirely voluntary and you can opt out at any time. Signing up to be a user research participant will have no impact on your feedback today, or your eligibility to access or receive services in the future.</p>", | |||
There was a problem hiding this comment.
Are we sure we want paragraph tags inside the translation files? I don't know if that would be considered best practice because it could interfere with the actual translation that goes on
There was a problem hiding this comment.
Yeah, I thought about this too, but I'm just not sure how else to customize the spacing between each paragraph of the body text (since design pointed out that there should be more spacing between each paragraph, but still keep it less than the spacing between the title and paragraph). I had considered just adding in another <br />, but we can't really customize how large that space is. I think another option would be to make the plainText an array, and then in Module.js map each piece of text into a <p> tag with our desired padding— not sure if this would be over-complicating things for our current purposes; it could become necessary if we ever end up having even more paragraphs per section in the future though. What do you think?
There was a problem hiding this comment.
I think we may want to find another solution. Additionally I don't know if we necessarily want
within our translations files either. Maybe something we should ask Rapi about.
There was a problem hiding this comment.
Yeah, I definitely agree that we should ask Rapi about this. Which part are you referring to in your second sentence?
There was a problem hiding this comment.
Sorry I meant line breaks within the files either
| <p | ||
| className={`${H1_WHITE_STYLE} ${dir === "rtl" && "text-right"}`} | ||
| className="font-sans-md2 feedback-h1 feedback-h1--light" |
There was a problem hiding this comment.
Do we need both "feedback-h1 and feedback-h1--light"? Isn't the purpose of using extends for --light so that it automatically inherits the styles from feedback-h1?
There was a problem hiding this comment.
I'd considered just using feedback-h1--light, but according to the official BEM docs, we should keep in the original class and add the new one separately (see http://getbem.com/naming/ Modifier section).
| p { | ||
| @extend .margin-0; | ||
| } | ||
| } |
There was a problem hiding this comment.
If two classes share a lot of styling in common can one of them extend the other so we don't have so much repetition within this file?
There was a problem hiding this comment.
Hmmm the thing with that is that if we have one extend the other it would extend all of the styling from the other (if we're also considering the uswds classes, it'd also extend the default uswds styling for one of those classes as well, that we didn't set). I think maybe an option to handle the common styling would be to use grouping selectors and move all the common styles in there?
For example
.usa-label, .usa-checkbox, .feedback-h1, etc {
@extend .text-bold, .text-primary;
}
What do you think?
There was a problem hiding this comment.
Yes I think that makes sense, I think we should do that
…s in order to avoid setting mobile-lg- and mobile-lg\: repeatedly
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://salmon-moss-01b532c0f-50.eastus2.azurestaticapps.net |
changed header logo from a png to an svg (help with resolution) added extra margin and padding to header logo so visually it appears centered
…es folder in case we want to support multiple themes for stretch goal, split components into different files for improved modularity
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://salmon-moss-01b532c0f-50.eastus2.azurestaticapps.net |
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://salmon-moss-01b532c0f-50.eastus2.azurestaticapps.net |
1 similar comment
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://salmon-moss-01b532c0f-50.eastus2.azurestaticapps.net |
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://salmon-moss-01b532c0f-50.eastus2.azurestaticapps.net |
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://salmon-moss-01b532c0f-50.eastus2.azurestaticapps.net |
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://salmon-moss-01b532c0f-50.eastus2.azurestaticapps.net |
