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.

Sign up for GitHub

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

Text boxes global settings setup #889

Merged
merged 2 commits into from
Nov 16, 2021
Merged

Text boxes global settings setup #889

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5b308ec
Text boxes global settings setup
tyleralsbury Nov 15, 2021
ac9a23a
Removed from gift card
tyleralsbury Nov 16, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions assets/section-image-banner.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,8 +197,13 @@
}

.banner__box {
border: 0;
padding: 4rem 3.5rem;
border: var(--text-boxes-border-width) solid rgba(var(--color-foreground), var(--text-boxes-border-opacity));
padding: var(--text-boxes-padding);
Copy link
Contributor

Choose a reason for hiding this comment

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

This is where trying to use a single pixel value rather than magic sizes to control multiple sections is going to be tricky. Or at least require a larger range. With banner_box previously designed at ~4rem, the current max for text boxes being 2rem is going to slim down those designs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. We may just want to expand the range for this one, too, but we'll need to look at the overall impact and make some decisions on that. We could do some magic, but that might be frustrating for a merchant who does want all text boxes to be the same.

cc @melissaperreault

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree with Ken. The Multicolumn section has some inner padding right now that doesn't allow the border to ever touch the text, while the banner section doesn't.

The border is very close to the text and touches the button(s) if they're present.
I think we'll need at least a little bit of default padding.

Screen Shot 2021-11-16 at 2 03 25 PM

Same for the password page:
Screen Shot 2021-11-16 at 2 15 57 PM

Copy link
Contributor Author

Choose a reason for hiding this comment

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

An alternative would be for it to be allowed to touch but to add overflow: hidden so that a merchant could create a text box with the buttons flush with the border - it could be an effect they're looking for.

border-radius: var(--text-boxes-radius);
box-shadow: var(--text-boxes-shadow-horizontal-offset)
var(--text-boxes-shadow-vertical-offset)
var(--text-boxes-shadow-blur-radius)
rgba(var(--color-foreground), var(--text-boxes-shadow-opacity));
position: relative;
height: fit-content;
align-items: center;
Expand Down Expand Up @@ -296,7 +301,6 @@

@media screen and (min-width: 750px) {
.banner__box {
padding: 5rem;
width: auto;
max-width: 71rem;
min-width: 45rem;
Expand Down
10 changes: 10 additions & 0 deletions assets/section-multicolumn.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,16 @@
}
}

.multicolumn-card {
border: var(--text-boxes-border-width) solid rgba(var(--color-foreground), var(--text-boxes-border-opacity));
padding: var(--text-boxes-padding);
border-radius: var(--text-boxes-radius);
box-shadow: var(--text-boxes-shadow-horizontal-offset)
var(--text-boxes-shadow-vertical-offset)
var(--text-boxes-shadow-blur-radius)
rgba(var(--color-foreground), var(--text-boxes-shadow-opacity));
}

.multicolumn-card__info .link {
text-decoration: none;
font-size: inherit;
Expand Down
101 changes: 101 additions & 0 deletions config/settings_schema.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,107 @@
}
]
},
{
"name": "Text boxes",
"settings": [
{
"type": "header",
"content": "Typography"
},
{
"type": "header",
"content": "Spacing"
},
{
"type": "range",
"id": "text_boxes_padding",
"min": 0,
"max": 20,
"step": 1,
"unit": "px",
"label": "Padding",
"default": 10
},
{
"type": "header",
"content": "Borders"
},
{
"type": "range",
"id": "text_boxes_border_opacity",
"min": 0,
"max": 100,
"step": 5,
"unit": "%",
"label": "Opacity",
"default": 50
},
{
"type": "range",
"id": "text_boxes_border_width",
"min": 0,
"max": 10,
"step": 1,
"unit": "px",
"label": "Thickness",
"default": 1
},
{
"type": "range",
"id": "text_boxes_radius",
"min": 0,
"max": 30,
"step": 3,
"unit": "px",
"label": "Corner radius",
"default": 0
},
{
"type": "header",
"content": "Shadow"
},
{
"type": "range",
"id": "text_boxes_shadow_opacity",
"min": 0,
"max": 100,
"step": 5,
"unit": "%",
"label": "Opacity",
"default": 0
},
{
"type": "range",
"id": "text_boxes_shadow_horizontal_offset",
"min": -20,
"max": 20,
"step": 2,
"unit": "px",
"label": "Horizontal offset",
"default": 0
},
{
"type": "range",
"id": "text_boxes_shadow_vertical_offset",
"min": -20,
"max": 20,
"step": 2,
"unit": "px",
"label": "Vertical offset",
"default": 0
},
{
"type": "range",
"id": "text_boxes_shadow_blur_radius",
"min": 0,
"max": 20,
"step": 2,
"unit": "px",
"label": "Blur radius",
"default": 0
}
]
},
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 already move these to the en file even if we arent requesting translations yet?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I find it easier to wait until we have the final copy and then do that, otherwise you need to go from one file to another to make the associations rather than just get them all in one place, then move them to their final spot.

{
"name": "t:settings_schema.typography.name",
"settings": [
Expand Down
9 changes: 9 additions & 0 deletions layout/password.liquid
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,15 @@

--page-width: {{ settings.page_width | divided_by: 10 }}rem;
--page-width-margin: {% if settings.page_width == '1600' %}2{% else %}0{% endif %}rem;

--text-boxes-padding: {{ settings.text_boxes_padding }}px;
--text-boxes-border-opacity: {{ settings.text_boxes_border_opacity | divided_by: 100.0 }};
--text-boxes-border-width: {{ settings.text_boxes_border_width }}px;
--text-boxes-radius: {{ settings.text_boxes_radius }}px;
--text-boxes-shadow-opacity: {{ settings.text_boxes_shadow_opacity | divided_by: 100.0 }};
--text-boxes-shadow-horizontal-offset: {{ settings.text_boxes_shadow_horizontal_offset }}px;
--text-boxes-shadow-vertical-offset: {{ settings.text_boxes_shadow_vertical_offset }}px;
--text-boxes-shadow-blur-radius: {{ settings.text_boxes_shadow_blur_radius }}px;
}
{% endstyle %}

Expand Down
11 changes: 10 additions & 1 deletion layout/theme.liquid
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,12 +90,21 @@
--popup-drawer-shadow-horizontal-offset: {{ settings.popup_drawer_shadow_horizontal_offset }}px;
--popup-drawer-shadow-vertical-offset: {{ settings.popup_drawer_shadow_vertical_offset }}px;
--popup-drawer-shadow-blur-radius: {{ settings.popup_drawer_blur_radius }}px;

{% comment %} Grid styles {% endcomment %}
--grid-desktop-vertical-spacing: {{ settings.grid_desktop_vertical_spacing | divided_by: 10.0 }}rem;
--grid-desktop-horizontal-spacing: {{ settings.grid_desktop_horizontal_spacing | divided_by: 10.0 }}rem;
--grid-mobile-vertical-spacing: {{ settings.grid_mobile_vertical_spacing | divided_by: 10.0 }}rem;
--grid-mobile-horizontal-spacing: {{ settings.grid_mobile_horizontal_spacing | divided_by: 10.0 }}rem;

--text-boxes-padding: {{ settings.text_boxes_padding }}px;
--text-boxes-border-opacity: {{ settings.text_boxes_border_opacity | divided_by: 100.0 }};
--text-boxes-border-width: {{ settings.text_boxes_border_width }}px;
--text-boxes-radius: {{ settings.text_boxes_radius }}px;
--text-boxes-shadow-opacity: {{ settings.text_boxes_shadow_opacity | divided_by: 100.0 }};
--text-boxes-shadow-horizontal-offset: {{ settings.text_boxes_shadow_horizontal_offset }}px;
--text-boxes-shadow-vertical-offset: {{ settings.text_boxes_shadow_vertical_offset }}px;
--text-boxes-shadow-blur-radius: {{ settings.text_boxes_shadow_blur_radius }}px;
}

*,
Expand Down
1 change: 1 addition & 0 deletions templates/gift_card.liquid
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@
--gradient-base-background-1: {% if settings.gradient_background_1 != blank %}{{ settings.gradient_background_1 }}{% else %}{{ settings.colors_background_1 }}{% endif %};

--page-width: {{ settings.page_width | divided_by: 10 }}rem;
--page-width-margin: {% if settings.page_width == '1600' %}2{% else %}0{% endif %}rem;
}
{% endstyle %}

Expand Down