c-smile / sciter-sdk

Sciter is an embeddable HTML/CSS/scripting engine
http://sciter.com
Other
2.11k stars 224 forks source link

Incorrect Responsive Grid Layout with Padding #189

Open borshpro opened 3 years ago

borshpro commented 3 years ago

Incorrect Responsive Grid Layout with Padding

Overview

Sciter incorrectly calculates container width, so it's impossible to get correctly displayed responsive layout. It looks like a bug, or I just misunderstand Sciter implementation.

Detailed

Currently I'm porting latest Bootstrap to Sciter with mixed success. Overall progress is very promising, but I failed with responsive grid layout of container, row and col (column) implementations.

Expected result

Proof

responsive-bug

Browser Result

Sciter Result

How to reproduce

Browser

  1. Load flex-html.htm into any modern browser
  2. Check output:
    • col-8 should be followed by col-4 in the same row (line).

Sciter

  1. Load flex-sciter.htm into Sciter test app (usciter).
  2. Check output:
    • output should match browser's version.
  3. Actual output:
    • sciter wraps elements that should fit 100%

HTML Code

cat flex-html.htm
<head>
    <title>Bootstrap test</title>
    <style>
    *,
    *::before,
    *::after {
        box-sizing: border-box;
    }
    body {
        margin: 0;
        font-size: 16px;
        color: #212529;
        text-align: left;
        background-color: #fff;
    }
    .container {
        width: 100%;
        padding-right: 15px;
        padding-left: 15px;
        margin-right: auto;
        margin-left: auto;
        max-width: 540px;
    }
    .row {
        display: flex;
        flex-wrap: wrap;
        margin-right: -15px;
        margin-left: -15px;
    }
    .col-4, .col-8 {
        position: relative;
        width: 100%;
        padding-right: 15px;
        padding-left: 15px;
    }
    .col-4 {
        flex: 0 0 33.333333%;
        max-width: 33.333333%;
    }
    .col-8 {
        flex: 0 0 66.666667%;
        max-width: 66.666667%;
    }
    </style>
</head>
<body>
<div >
    <div class="container">
        <div class="row">
            <div class="col-8">col-8</div>
            <div class="col-4">col-4</div>
        </div>
    </div>
</body>

Sciter Code

cat flex-sciter.htm
<head>
    <title>Bootstrap test</title>
    <style>
    *,
    *::before,
    *::after {
        box-sizing: border-box;
    }
    body {
        margin: 0;
        font-size: 16dip;
        color: #212529;
        text-align: left;
        background-color: #fff;
    }
    .container {
        width: 100%;
        padding-right: 15dip;
        padding-left: 15dip;
        margin-right: auto;
        margin-left: auto;
    }

    @media (min-width == 576dip) {
        .container, .container-sm {
            max-width: 540dip;
        }
    }

    .row {
        flow:horizontal-wrap;
        width:100%;
        margin-right: -15dip;
        margin-left: -15dip;
    }
    .col-4, .col-8 {
        position: relative;
        width: 100%;
        padding-right: 15dip;
        padding-left: 15dip;
    }

    .col-4 {
        width: 33.333333%;
        max-width: 33.333333%;
    }

    .col-8 {
        width: 66.666667%;
        max-width: 66.666667%;
    }
    </style>
    <script type="text/tiscript">

        function self.ready() {
            view.mediaVars
            {
                    min-width: 576dip,
            };
        }

    </script>
</head>
<body>
<div >
    <div class="container">
        <div class="row">
            <div class="col-8">col-8</div>
            <div class="col-4">col-4</div>
        </div>
    </div>
</body>

Sciter Version

Version 4.4.5.8
Revision 8356

System

OS Version Build
Windows 10 (2004) 10.0.19041.630
macOS 10 10.14.6

Request for detailed information

  1. How Sciter calculates relative sizes:
    • with margins
    • without margins
    • with paddings
    • without paddings
  2. Differences between Sciter's flex and HTML's flex in detail.
c-smile commented 3 years ago

Try this in latest Sciter and browser

<html>
<head>
  <title>Bootstrap test</title>
  <style>

  *,
  *::before,
  *::after {
    box-sizing: border-box;
  }
  body {
    margin: 0;
    font-size: 16px;
    color: #212529;
    text-align: left;
    background-color: #fff;
  }

  div.row {
    outline: 1px solid red;
    outline-offset: -1px;
  }
  div.row > div {
    outline: 1px solid blue;
    outline-offset: -1px;
  }

  /* for browser */
@supports (display:grid) { 

  .container {
    width: 100%;
    padding-right: 15px;
    padding-left: 15px;
    margin-right: auto;
    margin-left: auto;
    max-width: 540px;
  }
  .row {
    display: flex;
    flex-wrap: wrap;
    margin-right: -15px;
    margin-left: -15px;
  }
  .col-4, .col-8 {
    position: relative;
    width: 100%;
    padding-right: 15px;
    padding-left: 15px;
  }
  .col-4 {
    flex: 0 0 33.333333%;
    max-width: 33.333333%;
  }
  .col-8 {
    flex: 0 0 66.666667%;
    max-width: 66.666667%;
  }
}

@supports (flow:horizontal) { /* sciter */

  body { horizontal-align: center; }

  .container { width: 1*; }

  @media (min-width == 576px) {
    .container, .container-sm {
      max-width: 540px;
    }
  }

  .row { flow:horizontal-wrap; width:1*; }

  .col-4 { width: 0.33*; }
  .col-8 { width: 0.67*; }
}

  </style>
  <script type="text/tiscript">

    function self.ready() {
      view.mediaVars
      {
          min-width: 576px,
      };
    }

  </script>
</head>
<body>
  <div class="container">
    <div class="row">
      <div class="col-8">col-8</div>
      <div class="col-4">col-4</div>
    </div>
  </div>
</body>
</html>

And check specification:
https://sciter.com/docs/flex-flow/flex-layout.htm Or "TL;DR" : https://terrainformatica.com/w3/flex-layout/flex-vs-flexbox.htm

This https://sciter.com/developers/for-web-programmers/ may also be helpful.

borshpro commented 3 years ago

@c-smile, thank you for your sample code, but, unfortunately, proposed solution doesn't wrap columns as it should.

You could check your code with little modification (in browser and in Sciter), just duplicate <div class="col-4">col-4</div> line:

<body>
    <div class="container">
        <div class="row">
            <div class="col-8">col-8</div>
            <div class="col-4">col-4</div>
            <div class="col-4">col-4</div>
        </div>
    </div>
</body>

This will add one additional col-4 which should be wrapped as it exceeds 100% (67% + 33% + 33%).

Result in browser

Correct and expected behaviour in browser:

flex-browser-001

Result in Sciter

Sciter just adds another column which exceeds 100% but is shown unwrapped, on the same line, incorrect result:

flex-sciter-001

c-smile commented 3 years ago

Just add min constraint:

  .col-4 { width: 0.33*; min-width:200px; }
  .col-8 { width: 0.67*; min-width:200px; }

Or if you want to have exactly two elements per row then do

div.row > :nth-child(2n) { clear:after; }
borshpro commented 3 years ago

Andrew, I really appreciate your help, but I see several issues in your approach.

Intentions

I'll try to clarify my intentions:

  1. I need to use padding. It's in Issue's title.
  2. I need to use relative values instead of absolute ones.
  3. I want to use universal and compatible solutions, not hard-coded workarounds.

Proposed solutions

But what I see in your answers:

  1. You removed padding from sample code.
  2. You proposed to use fixed absolute values (min-width:200px;).
  3. You proposed hard-coded solution with n-th element processing (nth-child(2n)).

Problem

The main problem is: current flow/flex implementation in Sciter doesn't allow to use HTML-compatible logic (not exact code, only logic) without hard-coded workarounds.

It's important to normalise and synchronise Sciter logic with modern browser logic in internal width calculation (with paddings) to make it more consistent. This will allow to port real html code (e.g. Bootstrap) more seamlessly.

Proposal

If you haven't enough time to implement this, I could help you with your code. If I need a license for this (to access your source code), it's OK. For me it's more important to fix current issues with flow/flex logic to support more standard features in future. Sciter is a very solid and promising product and I'd like to make it even better.

c-smile commented 3 years ago

I need to use padding. It's in Issue's title.

So add padding. I've removed it to simplify the test.

You proposed to use fixed absolute values (min-width:200px;).

It could be anything, min-width:20vw; for example - I don't know your goals.

You proposed hard-coded solution with n-th element processing (nth-child(2n)).

That's just an idea, as I said I don't know what your final goal is. Are you trying to solve some concrete task or just testing how it can be done in general?

I do not have plans to implement display:flex as it breaks CSS box model and suspect it will be obsoleted at some point. But I do have plans for display:grid;.

c-smile commented 3 years ago

Yet I have plans to Open Source Sciter.JS next year so you will be able to try to implement display:flex there.

borshpro commented 3 years ago

I don't know your goals.

My goal is to port Bootstrap to Sciter while keeping intact Bootstrap logic. So there is no need to set artificial constraints like min-width if there is no such constraints in Bootstrap.