nunomaduro / termwind

🍃 In short, it's like Tailwind CSS, but for the PHP command-line applications.
MIT License
2.29k stars 78 forks source link

Gets rid of default tags <bg=default;options=> from console output #62

Closed butschster closed 3 years ago

butschster commented 3 years ago

One of the reason I decided to get rid of default tags is in this example

<div class="bg-red">Hello <strong>world></div>

When it parsed it looks like

<bg=red;options=>Hello <bg=default;options=bold>world</></>

As you noticed the word world has default background because of bg=default instead of red background.

There are a lot of unnecessary style tags in output :)

<br/> => <bg=default;options=></>\n
<a>link text</a> => <bg=default;options=>link text</>
<ol><li>list text 1</li></ol> => <bg=default;options=><bg=default;options=>1. list text 1</></>
xiCO2k commented 3 years ago

Thanks for this PR as well, it looks in the right way to inherit the styles of a parent element, just found one issue:

render(<<<'HTML'
    <div class="bg-red-300 text-color-white">
        <span class="mr-1">Hello</span>
        <strong class="text-color-blue">world</strong>
    </div>
HTML);

It renderes this:

image

Looks like when there is a text color it ignores the parent bg color.

All the rest looks like a great addition.

butschster commented 3 years ago

Thanks for this PR as well, it looks in the right way to inherit the styles of a parent element, just found one issue:

render(<<<'HTML'
    <div class="bg-red-300 text-color-white">
        <span class="mr-1">Hello</span>
        <strong class="text-color-blue">world</strong>
    </div>
HTML);

It renderes this:

image

Looks like when there is a text color it ignores the parent bg color.

All the rest looks like a great addition.

Hmm. I bumped into a similar issue when I was working on the table parser.

I was thinking that we can pass styles from parent element to child elements and mergee them

Something like this

public static function div(array|string $content = '', string $styles = ''): Components\Div
{
        $content = implode('', array_map(static funtion ($element) use($styles) {
                // I think we need to merge styles somewere here.
                // Moreover we don't need to replace existing styles, only add new from parent
                $element->inheritFromStyles($styles);

                return (string) $element;
        }, is_array($content) ? $content : [$content]));

        return Components\Div::fromStyles(
            self::getRenderer(), $content, $styles
        );
 }
xiCO2k commented 3 years ago

Yes we definitely need something to handle the inherit styles.

cc @nunomaduro.

butschster commented 3 years ago

Yes we definitely need something to handle the inherit styles.

cc @nunomaduro.

I have an idea how to fix it and the idea can fix this issue #53 I think. I need to try it.

butschster commented 3 years ago

Yes we definitely need something to handle the inherit styles.

cc @nunomaduro.

Take a look at #64 PR. I tried to solve issue with inheritance and fized #53 issue there.

It has a draft state. Mayebe there will be some changes and proposals from you.

I covered with tests all issues.

Feel free to modify code form the PR

xiCO2k commented 3 years ago

Thanks @butschster, will take a look on it.

butschster commented 3 years ago

Hi @nunomaduro. Is there any chance you approve this pr? I need then to refactor pr with table and stast using the packages.

nunomaduro commented 3 years ago

Thank you, @butschster !

butschster commented 3 years ago

Thank you, @butschster !

Thank you too!