Skip to content

feature: Added font-weight support #165

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

Merged
merged 6 commits into from
Nov 6, 2022
Merged

feature: Added font-weight support #165

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
427f06f
feat: add font weight to form
it-jhack Nov 6, 2022
62486e6
feat: add font-weight params and default to functions
it-jhack Nov 6, 2022
a65ee59
Merge branch 'feat-font-weight' into main
it-jhack Nov 6, 2022
8720630
feat: add min, max, step to font-weight
it-jhack Nov 6, 2022
8355424
feat: add font-weight default on demo script
it-jhack Nov 6, 2022
41fc745
Merge branch 'feat-font-weight' into main
it-jhack Nov 6, 2022
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
3 changes: 3 additions & 0 deletions src/demo/index.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,9 @@ function gtag() {
</div>
<input class="param" type="text" id="font" name="font" alt="Font name" placeholder="Fira Code" value="Fira Code" pattern="^[A-Za-z0-9\- ]*$" title="Font from Google Fonts. Only letters, numbers, and spaces.">

<label for="weight">Font weight</label>
<input class="param" type="number" id="weight" name="weight" alt="Font weight" placeholder="400" value="400" min="100" max="900" step="100">

<label for="size">Font size</label>
<input class="param" type="number" id="size" name="size" alt="Font size" placeholder="20" value="20">

Expand Down
1 change: 1 addition & 0 deletions src/demo/js/script.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ let preview = {
// default values
defaults: {
font: "monospace",
weight: "400",
color: "36BCF7",
background: "00000000",
size: "20",
Expand Down
4 changes: 2 additions & 2 deletions src/models/GoogleFontConverter.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,12 @@ class GoogleFontConverter
* @param string $text Text to display in font
* @return string|false The CSS for displaying the font
*/
public static function fetchFontCSS($font, $text)
public static function fetchFontCSS($font, $weight, $text)
{
$url =
"https://fonts.googleapis.com/css2?" .
http_build_query([
"family" => $font,
"family" => $font . ":wght@" . $weight,
"text" => $text,
"display" => "fallback",
]);
Expand Down
11 changes: 8 additions & 3 deletions src/models/RendererModel.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@ class RendererModel
/** @var string $font Font family */
public $font;

/** @var string $font Font weight */
public $weight;

/** @var string $color Font color */
public $color;

Expand Down Expand Up @@ -52,6 +55,7 @@ class RendererModel
/** @var array<string, string> $DEFAULTS */
private $DEFAULTS = [
"font" => "monospace",
"weight" => "400",
Copy link
Owner

@DenverCoder1 DenverCoder1 Nov 6, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

In demo/js/script.js there is also a place to add defaults for the demo site that makes the URL shorter when the default value is used.

If you could add weight: "400", to the defaults object in that file as well that would be great. You can edit that on your forked branch.

(Btw for the future, it's typically better to create a new branch in your fork before working on a feature, rather than pushing to main since it allows you to have main kept up to date with the original repo and work on multiple features on separate branches. It's totally fine in this case, just a tip for going forward 😄).

Copy link
Contributor Author

@it-jhack it-jhack Nov 6, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Will do!
Thank you very much for the detailed feedback. I actually did work on a separate branch "feat-font-weight" that had only 2 commits, but I merged it to my "main" when I finished and used that for the PR. So the ideal would be to PR my "feat-font-weight" directly to your "main"?

Copy link
Owner

Choose a reason for hiding this comment

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

Yeah, best is to create a branch for the PR and create the PR from that feature branch to the upstream repo's main. Then updating that feature branch will also update the PR.

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 just updated the demo default on a new branch and merged into my main, not sure if I did it the proper way 😬

Copy link
Owner

@DenverCoder1 DenverCoder1 Nov 6, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

As long as it all ends up on the right branch eventually, it's fine.

image

In this case the source branch of the PR is main, so all new commits to the PR would go there.

If you created the PR from a separate branch, then that separate branch would be where the new commits go.

"color" => "#36BCF7",
"background" => "#00000000",
"size" => "20",
Expand All @@ -75,6 +79,7 @@ public function __construct($template, $params)
$this->template = $template;
$this->lines = $this->checkLines($params["lines"] ?? "");
$this->font = $this->checkFont($params["font"] ?? $this->DEFAULTS["font"]);
$this->weight = $this->checkNumberPositive($params["weight"] ?? $this->DEFAULTS["weight"], "Font weight");
$this->color = $this->checkColor($params["color"] ?? $this->DEFAULTS["color"], "color");
$this->background = $this->checkColor($params["background"] ?? $this->DEFAULTS["background"], "background");
$this->size = $this->checkNumberPositive($params["size"] ?? $this->DEFAULTS["size"], "Font size");
Expand All @@ -85,7 +90,7 @@ public function __construct($template, $params)
$this->multiline = $this->checkBoolean($params["multiline"] ?? $this->DEFAULTS["multiline"]);
$this->duration = $this->checkNumberPositive($params["duration"] ?? $this->DEFAULTS["duration"], "duration");
$this->pause = $this->checkNumberNonNegative($params["pause"] ?? $this->DEFAULTS["pause"], "pause");
$this->fontCSS = $this->fetchFontCSS($this->font, $params["lines"]);
$this->fontCSS = $this->fetchFontCSS($this->font, $this->weight, $params["lines"]);
}

/**
Expand Down Expand Up @@ -185,12 +190,12 @@ private function checkBoolean($bool)
* @param string $text Text to display in font
* @return string The CSS for displaying the font
*/
private function fetchFontCSS($font, $text)
private function fetchFontCSS($font, $weight, $text)
{
// skip checking if left as default
if ($font != $this->DEFAULTS["font"]) {
// fetch and convert from Google Fonts
$from_google_fonts = GoogleFontConverter::fetchFontCSS($font, $text);
$from_google_fonts = GoogleFontConverter::fetchFontCSS($font, $weight, $text);
if ($from_google_fonts) {
// return the CSS for displaying the font
return "<style>\n{$from_google_fonts}</style>\n";
Expand Down