support letter spacing #54

You-J · 2021-10-14T13:47:20Z

wip

vercel · 2021-10-14T13:47:23Z

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/grida/designto-codes/2GD8iK6M36r6m6ZnadzEjAokAB1y
✅ Preview: https://designto-codes-git-support-letter-spacing-grida.vercel.app

CLAassistant · 2021-10-14T13:47:27Z

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

softmarshmallow · 2021-10-15T14:01:59Z

packages/builder-css-styles/dimensions/px.ts

    // making to fixed number since css does not support decimal px points.
-    return `${d.toFixed()}px`;
+    // If there is a decimal point, it is rounded up to the first place.


Explain this

If there is a decimal point in css, the first digit is rounded off instead of rounding. Upload the reference written in that file.

https://stackoverflow.com/a/4309051

It's round, but it's round up. I will fix it to round

I mean what is the whole point of altering the value?
Why round at all in the first place

You know that toFixed and round basically does the same thing right?

Confirmed, I was mistaken. sorry.

softmarshmallow · 2021-10-15T14:02:23Z

packages/builder-web-core/widgets-native/html-text-span/index.ts

@@ -66,6 +65,12 @@ export class Text extends TextChildWidget {
        "line-height": css.length(this.textStyle.lineHeight),
      };
    }
+    if (!!this.textStyle.letterSpacing) {
+      textStyle = {
+        ...textStyle,


Invalid syntax

Exception handling when default is 0, what's the problem?

https://github.com/gridaco/designto-code/blob/c9b77d1b94685208067fe3d375cd5ca8cc8f383a/packages/builder-web-core/widgets-native/flex/index.ts#L117

This does not make sense at all.
Never do this again

I checked I'll fix it!

softmarshmallow · 2021-10-15T14:04:54Z

packages/designto-flutter/length/multiple.ts

@@ -7,7 +7,7 @@ export function multiple(origin: number, target: DimensionLength) {
    const targetToNum = parseInt(target.match(regx)[0]);


Explain this file

When the target is DimensionLength type, this function measures the multiple from the origin number.

That's now what i meant by "explain".
Explain why you did this way

Are you asking because of regx? We've talked about this before. I'll fix this too.

softmarshmallow · 2021-10-15T14:18:02Z

packages/builder-css-styles/dimensions/px.ts

    // making to fixed number since css does not support decimal px points.
-    return `${d.toFixed()}px`;
+    // If there is a decimal point, it is rounded up to the first place.


I mean what is the whole point of altering the value?
Why round at all in the first place

softmarshmallow · 2021-10-15T14:20:42Z

packages/builder-css-styles/dimensions/px.ts

    // making to fixed number since css does not support decimal px points.
-    return `${d.toFixed()}px`;
+    // If there is a decimal point, it is rounded up to the first place.


You know that toFixed and round basically does the same thing right?

softmarshmallow · 2021-10-15T14:24:34Z

packages/builder-web-core/widgets-native/html-text-span/index.ts

@@ -66,6 +65,12 @@ export class Text extends TextChildWidget {
        "line-height": css.length(this.textStyle.lineHeight),
      };
    }
+    if (!!this.textStyle.letterSpacing) {
+      textStyle = {
+        ...textStyle,


https://github.com/gridaco/designto-code/blob/c9b77d1b94685208067fe3d375cd5ca8cc8f383a/packages/builder-web-core/widgets-native/flex/index.ts#L117

softmarshmallow · 2021-10-15T14:25:03Z

packages/designto-flutter/length/multiple.ts

@@ -7,7 +7,7 @@ export function multiple(origin: number, target: DimensionLength) {
    const targetToNum = parseInt(target.match(regx)[0]);


That's now what i meant by "explain".
Explain why you did this way

softmarshmallow · 2021-10-15T14:26:21Z

packages/builder-web-core/widgets-native/html-text-span/index.ts

@@ -66,6 +65,12 @@ export class Text extends TextChildWidget {
        "line-height": css.length(this.textStyle.lineHeight),
      };
    }
+    if (!!this.textStyle.letterSpacing) {
+      textStyle = {
+        ...textStyle,


This does not make sense at all.
Never do this again

softmarshmallow · 2021-10-15T14:35:29Z

packages/designto-flutter/painting/painting-text-style.ts

  return new flutter.TextStyle({
    fontSize: rd(style.fontSize),
    fontWeight: fontWeight,
    fontFamily: fontFamily,
    color: dartui.color(style.color),
    fontStyle: fontStyle(style.fontStyle),
-    letterSpacing: letterSpacing, // percentage is not supported
+    letterSpacing: rd(letterSpacing as number), // percentage is not supported


what's the as number for

softmarshmallow · 2021-10-18T08:22:18Z

packages/builder-css-styles/dimensions/px.ts

@@ -2,7 +2,10 @@ export function px(d: number): string | undefined {
  if (d === undefined || d === null) {
    return;
  } else {
+    // https://stackoverflow.com/questions/4308989/are-the-decimal-places-in-a-css-width-respected
    // making to fixed number since css does not support decimal px points.


softmarshmallow · 2021-10-18T08:23:12Z

packages/builder-web-core/widgets-native/html-text-span/index.ts

@@ -66,6 +65,12 @@ export class Text extends TextChildWidget {
        "line-height": css.length(this.textStyle.lineHeight),
      };
    }
+    if (!!this.textStyle.letterSpacing) {
+      textStyle = {
+        ...textStyle,


stash support letter spacing

0d16d42

fix: round up to one decimal place in px function

0799867

vercel bot had a problem deploying to Preview October 14, 2021 18:29 Failure

handling using multiple for letterSpacing in flutter

f109651

vercel bot had a problem deploying to Preview October 14, 2021 23:36 Failure

fix: letter spacing handle to rd in flutter

a40fe8e

vercel bot deployed to Preview October 15, 2021 12:10 View deployment

You-J added 2 commits October 15, 2021 22:40

exclude letter spacing styles when default

cfecb2a

fix multiple function when target unit %

af08766

vercel bot deployed to Preview October 15, 2021 13:47 View deployment

softmarshmallow requested changes Oct 15, 2021

View reviewed changes

fix: px function round up to round

972cd81

vercel bot deployed to Preview October 15, 2021 14:32 View deployment

fix multiple handling

8affebc

vercel bot deployed to Preview October 18, 2021 04:45 View deployment

Added handler due to letterapcing type change

ed70ad2

vercel bot deployed to Preview October 18, 2021 07:58 View deployment

add comment

9411573

vercel bot deployed to Preview October 18, 2021 08:02 View deployment

You-J merged commit 003fbf3 into main Oct 19, 2021

softmarshmallow reviewed Oct 19, 2021

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

support letter spacing #54

support letter spacing #54

You-J commented Oct 14, 2021

vercel bot commented Oct 14, 2021 •

edited

CLAassistant commented Oct 14, 2021

softmarshmallow Oct 15, 2021

You-J Oct 15, 2021

You-J Oct 15, 2021

softmarshmallow Oct 15, 2021

softmarshmallow Oct 15, 2021

You-J Oct 19, 2021

softmarshmallow Oct 15, 2021

You-J Oct 15, 2021

softmarshmallow Oct 15, 2021

softmarshmallow Oct 15, 2021

softmarshmallow Oct 18, 2021

You-J Oct 19, 2021

softmarshmallow Oct 15, 2021

You-J Oct 15, 2021

softmarshmallow Oct 15, 2021

You-J Oct 19, 2021

softmarshmallow Oct 15, 2021

softmarshmallow Oct 15, 2021

softmarshmallow Oct 15, 2021

softmarshmallow Oct 15, 2021

softmarshmallow Oct 15, 2021

softmarshmallow Oct 15, 2021

softmarshmallow Oct 18, 2021

softmarshmallow Oct 18, 2021

		@@ -7,7 +7,7 @@ export function multiple(origin: number, target: DimensionLength) {
		const targetToNum = parseInt(target.match(regx)[0]);

support letter spacing #54

support letter spacing #54

Conversation

You-J commented Oct 14, 2021

vercel bot commented Oct 14, 2021 • edited

CLAassistant commented Oct 14, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vercel bot commented Oct 14, 2021 •

edited