pressbooks / buckram

Opinionated SCSS components for books (web, EPUB and PDF).
https://buckram.pressbooks.org
GNU General Public License v3.0
4 stars 4 forks source link

.textbox--sidebar does not use defined variable $textbox-sidebar-max-width #327

Closed beckej13820 closed 1 year ago

beckej13820 commented 1 year ago

Prerequisites

Description

If this is incorrect, please forgive. I'm trying to create a custom child theme and teach myself SCSS and buckram at the same time.

Buckram has a defined variable $textbox-sidebar-max-width: 25% !default; https://buckram.pressbooks.org/#variable-textbox-sidebar-max-width

However, in buckram/assets/styles/components/specials/_textboxes.scss the max-width value is hardcoded at 25%.

Screenshot 2023-06-17 at 7 39 40 PM

SteelWagstaff commented 1 year ago

@greatislander this looks like a good catch and easy fix for us to make, unless I'm missing something? We should remove line 215, no?

beckej13820 commented 1 year ago

I think instead of deleting that line, line 215 should read

max-width: $textbox-sidebar-max-width;

that should set it to the variable which is by default to 25% and but allow us to do overrides.

SteelWagstaff commented 1 year ago

Ah, that makes more sense! Do you want to open a PR to that effect, Ed? Here's a short primer for how to make very simple PRs like this in case you want to give it a shot? https://www.youtube.com/watch?v=fRaHq5jd2Po

beckej13820 commented 1 year ago

I watched your video, and I'd like to try to do that. Do I have to assign a reviewer? If I don't assign a reviewer, does it go into a pool and notify you in some way?

SteelWagstaff commented 1 year ago

You don’t have to assign a reviewer, we can handle that. If you’d like, however, you are welcome to assign me as the reviewer.Please forgive any typos, as this message was typed on a screen. On Jun 30, 2023, at 5:25 AM, beckej13820 @.***> wrote: I watched your video, and I'd like to try to do that. Do I have to assign a reviewer? If I don't assign a reviewer, does it go into a pool and notify you in some way?

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were assigned.Message ID: @.***>

SteelWagstaff commented 1 year ago

Two things to note: the branch name needs to follow this pattern: fix/textbox-sidebar and the PR title should be something like: fix: use text-sidebar width variable

beckej13820 commented 1 year ago

Thanks Steel, I've made those two changes.

Is that a Pressbooks naming convention or an everybody-in-the-world naming convention?

greatislander commented 1 year ago

@beckej13820: it's based on this if I'm not mistaken: https://www.conventionalcommits.org/en/v1.0.0/#summary

SteelWagstaff commented 1 year ago

@beckej13820 yes, Ned is correct. We've recently started enforcing those Conventional Commits conventions on branches and PR titles/commits -- once we're doing this consistently it will allow us to automate (and improve the speed and accuracy of) our release notes for various projects.