dringtech / ifg-charts

Visualisation component library for IfG
MIT License
1 stars 0 forks source link

Excess whitespace where only one line of footer text #7

Closed philipnye closed 1 week ago

philipnye commented 4 months ago

Where there's only a single line of footer text there is too much whitespace between the bottom of the footer text/the CC logo and the bottom of the SVG. The amount of whitespace should be similar to the ammount between the top of the footer text/the CC logo and the blue bar (I think this is approx 11px in our static charts), with the footer section increasing in height with additional lines of text, following the logic of the header.

gilesdring commented 4 months ago

There was a minimum footer height, which I've altered the implementation of.

sachinsavur commented 3 months ago

Still seem to be excess whitespace where there's only one line of text - do we need to decrease the minimum footer height further? timeline (31)

gilesdring commented 2 months ago

Not seeing this on the test version of the component.

image

It may be that it's not in the version of the library on the site, or could be some CSS that's overriding the styling. Propose that it's re-tested when the next version has been released.

philipnye commented 1 month ago

@gilesdring Unfortunately we've tested on v0.9 and we're still seeing this. It looks to be the SVG element itself that's too tall rather than the g element containing the footer elements. Do you have any idea what selectors we might be talking about?

graham73may commented 1 month ago

Taking a look at the chart embedded into the page, the SVG element has some spacing at the bottom in the browser.

image

I've exported as PNG:

timeline

And exported as SVG:

timeline

graham73may commented 1 month ago

Just taking a look at the elements inside the tag they seem to be the correct size (see lime green outline below), but the SVG element is taller than them.

image

It's like the viewBox has 20px too many, I can't see anything that may be causing it from our side (although I don't really know how the SVG is being drawn).

gilesdring commented 1 week ago

The problem is that there are actually two lines being sent to the component, the second one is an empty string. The code is calculating the footer height based on the length of the array. To fix this I could either strip out empty strings from the array, or they could be stripped out before injecting into the notes field of the component.

graham73may commented 1 week ago

Hi @gilesdring, any idea if this would this be something coming from Philip's SQL output? Or something from my side?

Do you have an example of the problem string/data, it might help me track it down, thanks!

gilesdring commented 1 week ago

Hi @graham73may - the notes option is derived from this statement in the web/modules/custom/ifg_ministers/assets/js/ifg-ministers-initialise-chart.js code.

                // 5. Use the ganttData to set up our chart.
                const {
                    data,
                    overlay,
                    startDate,
                    endDate,
                    title,
                    source,
                    notes,
                } = ganttData;

Looks like this notes is being concatenated to the source string as an array and passed in as the notes field to the chart.

   notes: [source, notes],

Both these elements are set (as in web/modules/custom/ifg_ministers/src/Form/MinistersPeopleWhoHeldRoleForm.php) by a call like this:

    $gant_metadata = $this->queryService->queryForGanttMeta($arguments);

I think this means that whatever the values of those strings, the array will have a length of two... A quick fix would be to filter in the web/modules/custom/ifg_ministers/assets/js/ifg-ministers-initialise-chart.js, something like:

   notes: [source, notes].filter(x => x),

This would remove any empty values from the array, and the spacing would work OK.

gilesdring commented 1 week ago

@graham73may I'll make this change in the Drupal site... Will also put in a fix for #16 at the same time!

gilesdring commented 1 week ago

This change in drupal site should resolve this.

https://gitlab.com/sb-dev-team/instituteforgovernment.org.uk/-/commit/d4d600793fe2659eeb2c4041c9724bbf5a903fd6

graham73may commented 1 week ago

This is now deployed, spacing issue is resolved for me. Example png attached.

timeline (4)