highcharts / highcharts

Highcharts JS, the JavaScript charting framework
https://www.highcharts.com
Other
11.87k stars 3.54k forks source link

dash/non-nested-element #21150

Closed karolkolodziej closed 3 weeks ago

karolkolodziej commented 1 month ago

Added isStandalone option to add non-nested options in the edit mode sidebar, #20793.

highsoft-bot commented 1 month ago

File size comparison

Sizes for compiled+gzipped (bold) and compiled files. master candidate difference
dashboards/dashboards.js 43.1 kB
148.4 kB
43.1 kB
148.4 kB
11 B
41 B
dashboards/modules/layout.js 11.7 kB
45.8 kB
11.7 kB
45.9 kB
40 B
55 B
highsoft-bot commented 1 month ago

Lighthouse report

dashboards-demo-minimal.json

Reference Proposed Diff
performance – score 1 1 0.00
first-contentful-paint – score 1 1 0.00
first-contentful-paint – milliseconds 230.07 189.25 -40.82
first-meaningful-paint – score 1 1 0.00
first-meaningful-paint – milliseconds 451.08 437.64 -13.44
dom-size – score 1 1 0.00
dom-size – elements 346 346 0.00
highsoft-bot commented 1 month ago

Dashboard visual diffs

No differences found

highsoft-bot commented 1 month ago

Visual test results - No difference found

highsoft-bot commented 1 month ago

Benchmark report - Dashboards

benchmarks/Dashboards/DataPool-CSV-constructor.bench.ts

Sample size This PR avg (ms) master avg (ms) Diff Percent diff
2500000 0.16 0.16 0 1%
See all | Sample size | This PR avg (ms) | master avg (ms) | Diff | Percent diff | | --- | --- | --- | --- | --- | | 100 | 0.16 | 0.16 | 0 | 1% | 1000 | 0.16 | 0.17 | 0 | -2% | 10000 | 0.17 | 0.16 | 0 | 2% | 100000 | 0.17 | 0.16 | 0.01 | 3% | 1000000 | 0.16 | 0.17 | 0 | -3% | 2500000 | 0.16 | 0.16 | 0 | 1%

benchmarks/Dashboards/DataTable-loading-columns.bench.ts

Sample size This PR avg (ms) master avg (ms) Diff Percent diff
2500000 16.32 15.79 0.52 3%
See all | Sample size | This PR avg (ms) | master avg (ms) | Diff | Percent diff | | --- | --- | --- | --- | --- | | 100 | 0.24 | 0.24 | 0 | 0% | 1000 | 0.24 | 0.24 | 0.01 | 3% | 10000 | 0.27 | 0.27 | 0 | 1% | 100000 | 2.29 | 2.41 | -0.12 | -5% | 1000000 | 8.79 | 9.05 | -0.26 | -3% | 2500000 | 16.32 | 15.79 | 0.52 | 3%

benchmarks/Dashboards/DataTable-loading-rows.bench.ts

Sample size This PR avg (ms) master avg (ms) Diff Percent diff
2500000 745.75 742.54 3.21 0%
See all | Sample size | This PR avg (ms) | master avg (ms) | Diff | Percent diff | | --- | --- | --- | --- | --- | | 100 | 0.28 | 0.29 | 0 | 0% | 1000 | 0.53 | 0.51 | 0.02 | 3% | 10000 | 3.16 | 3.08 | 0.08 | 3% | 100000 | 16.55 | 18.14 | -1.59 | -9% | 1000000 | 287.82 | 287.07 | 0.75 | 0% | 2500000 | 745.75 | 742.54 | 3.21 | 0%
stitot commented 4 weeks ago

Hm..... We are adding some new concepts/wording here (nested, standalone) in addition to new option. I feel it's perhaps to verbose. Not possible with a simpler approach? E.g.

editableOptions: [{
        name: 'Component title',
        propertyPath: ['title'],
        type: 'input'
      }, {
        name: 'chartOptions',
        children: [{
          name: 'Marker Radius',
          options: [{
            name: 'Marker Radius',
            propertyPath: [
              'chartOptions',
              'plotOptions',
              'series',
              'marker',
              'radius'
            ],
            type: 'select',
            selectOptions: [{
              name: 3
            }, {
              name: 5
            }]
          }]
        }]
      }
    ]

If the children option doesn't exist it's standalone (the default). Isn't children a more common terminology for this concept, from a dev point of view? If feel standalone/nested calls for more explanation.

karolkolodziej commented 3 weeks ago

@stitot I'm fine with that but the only drawback is that with this approach it won't be possible to have collapsible singular elements like we used to have: image

stitot commented 3 weeks ago

Hm...... From a UX (and perhaps a11y?) perspective it makes sense for me to have all these single values exposed. Takes more space, but easier to find the value you try to edit (by looking at actual value, not name of field) instead of expanding them to see what's hidden inside.

We could introduce an collapsed option, that is false by default?