Fix overflowing advanced styling panel in Map Layer Manager

[CWDamm-Kint]

Types of changes

• Bugfix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)

Description of Change

When the advanced styling config is not properly formatted, the advanced styling panel overflows the screen. There is an option for codemirror to wrap text, which stops this from happening.

Issues Solved

Closes #11649

Checklist

• I targeted one of these branches:
  • dev/8.0.x (under development): features, bugfixes not covered below
  • dev/7.6.x (main support): regressions, crashing bugs, security issues, major bugs in new features
  • dev/6.2.x (extended support): major security issues, data loss issues
• I added a changelog in arches/releases
• I submitted a PR to arches-docs (if appropriate)
• Unit tests pass locally with my changes
• I added tests that prove my fix is effective or that my feature works
• My test fails on the target branch

Accessibility Checklist

Developer Guide

Topic	Changed	Retested
Color contrast
Form fields
Headings
Links
Keyboard
Responsive Design
HTML validation
Screen reader

Ticket Background

• Sponsored by: Knowledge Integration
• Found by: @CWDamm-Kint
• Tested by: @CWDamm-Kint
• Designed by: @CWDamm-Kint

Further comments

NA

[chiatt]

@CWDamm-Kint, I've been unable to reproduce this bug in 7.6. Could you narrow down the conditions that cause it?

[CWDamm-Kint]

@chiatt, I've updated the issue with a clip from 7.6 while pasting in improperly formatted json. Arguably, this is somewhat unlikely as Arches produces properly formatted json for new models by default, and you would likely then just edit what's there. But perhaps worth having the fix as a back up if something similar did recur?

[chiatt]

Thanks @CWDamm-Kint - That's helpful. I'm wondering if this fix, although it certainly makes the data more readable, isn't masking the actual problem. My guess is that if you make a change to the style, like a change of a fill color, that change won't be reflected in the map. I suspect that the data is getting saved as a json-like string rather than json. If that's the case, then the fix that's really needed is ensuring that the data gets converted to true json on save.

[SDScandrettKint]

Thanks @CWDamm-Kint - That's helpful. I'm wondering if this fix, although it certainly makes the data more readable, isn't masking the actual problem. My guess is that if you make a change to the style, like a change of a fill color, that change won't be reflected in the map. I suspect that the data is getting saved as a json-like string rather than json. If that's the case, then the fix that's really needed is ensuring that the data gets converted to true json on save.

It sounds like there are two issues at play here: the styling causing an overflow, and the incorrect AfHER data; but the overflow issue is caused by the json-like string in the AfHER data.

I can trace the json string down to the AfHER models, here: https://github.com/archesproject/arches-her/blob/a8e9144fc622921e02e3e0fb28a48bc711b58851/arches_her/pkg/graphs/resource_models/Application%20Area.json#L8184. I can create this as an issue in AfHER.

The line wrapping seems like a small enough addition to fix the visual side of this issue, but the data is truly the culprit.

[aj-he]

there are two issues at play here: the styling causing an overflow, and the incorrect AfHER data; but the overflow issue is caused by the json-like string in the AfHER data.

@SDScandrettKint - This string will have been generated by the system on exporting the graph. The content of the string passes validation as JSON if you remove the escape characters. This seems to imply that there could be an underlying issue with how this json-like string is loaded from the model file? Or, how the string is loaded into codemirror?

[SDScandrettKint]

The AfHER json string must be failing to JSON stringify somewhere then?

The AfHER advancedStyle JSON is functional. Changing some of the fill colours is reflected on the map.

I've also noticed that any string can be added to the advanced styling box, it is not strict on JSON. At first, this was tested by replacing the AfHER model advancedStyle line with regular text, but it looks as if this can also be done through the UI.
As an example for a long string, I pasted one of the paragraphs from archesproject.org into the advanced styling and it produced the same issue (see below).

overflow_advanced_styling.mp4

EDIT: console logging config.advancedStyle returns a string always, regardless of if it is JSON formatted in the advanced style box. Should we be converting the value to JSON somewhere?

[chiatt]

If a user modifies the Advanced Styling in the UI, the style is saved as a formatted string:

It's not very pretty, but codemirror can load this and it render it as formatted 'json'. The issue for HER is that the tab and newline characters have all been removed, so codemirror is rendering the data exactly as expected - as a single line.

I tested Advanced Styling with properly formatted json by exporting a graph, cleaning up the data and reloading it. The data loaded as valid json in the database, but codemirror threw an error when I tried to load it in Advanced Styling. I suspect the issue isn't with codemirror, but with how it's being used. Unfortunately a fix for this (so that only valid json is managed) might mean cleaning up data that is not formatted as valid json and most likely means a migration to do it. For the time being @CWDamm-Kint's solution addresses the problem of being unable able to edit Advanced styling in cases such as Arches for HERs.

[SDScandrettKint]

@chiatt I have raised a new ticket with this issue that you encountered - I'll try reproduce.

I think I encountered another issue from looking into this bug - changing advanced styling config doesn't reflect the changes in the UI user-input boxes e.g. changing polygon paint colour. Now this could be due to me changing the wrong value, but it might need to be looked into.