Conversation
| "pie": "Mostra legenda", | ||
| "bar": "Mostra legenda", | ||
| "gauge": "Mostra etichette" | ||
| }, |
There was a problem hiding this comment.
use google translate for the translations for it, es, fr, de
the other languages you can leave english
web/client/translations/data.it-IT
Outdated
| "displayCartesian": { | ||
| "line": "Hide Grid", | ||
| "bar": "Hide Grid" | ||
|
|
There was a problem hiding this comment.
Yes please, remove the white line. We should keep the same line numbers in all translation files
| @@ -0,0 +1,16 @@ | |||
| const React = require('react'); | |||
|
|
|||
| const AxisLabel = ({ axisType, x, y, width, height, stroke, children }) => { | |||
There was a problem hiding this comment.
- add/create/modify unit tests to check your changes to the code
- The vertical Y axis you added looks good, but sometimes doesn't fit. It is better to remove it.
- The custom Y axis label should be the one in the screenshot:
- Please remember to replace the Y axis name even in the table column
- When you edit an existing widget, the advanced option panel don't display Y axis label
web/client/translations/data.it-IT
Outdated
| "displayCartesian": { | ||
| "line": "Hide Grid", | ||
| "bar": "Hide Grid" | ||
|
|
There was a problem hiding this comment.
Yes please, remove the white line. We should keep the same line numbers in all translation files
| @@ -0,0 +1,39 @@ | |||
| /* chart advance options */ | |||
There was a problem hiding this comment.
In order to avoid the creation of too mani less files, you should move this into the widget.less, under the section "BUILDER"
applied reviewed comments
…into charts_options
offtherailz
left a comment
There was a problem hiding this comment.
- Y axis numbers option must be present
- Also the chart legend should be replaced with the custom label
There was a problem hiding this comment.
There are still a lot of issues:
- When I create a new chart, when I open the advanced options panel I can not see the same settings I see in the preview:
- hide grid is on, but the grid is visible.
- hide y axis is switched off, so I should see the labels, but I can't.
- Changing the label text with display legend turned on do not change the legend in the preview chart.
- also that you named "label" the y axis presence flag, and "yAxisLabel" the label value in options.
Please rename variables accordingly to their purpose. - I see WPSWidgetOptions is not well formatted (from line 164 is very confusing) . Please make it readable
|
regarding " Changing the label text with display legend turned on do not change the legend in the preview chart ". |
|
You shouldn't reload the chart if you're changing the |
|
would a submit button, that onSubmit changes the label state work? something that has the massage "insert " for example? |
|
No, take a look on how it is implemented for unit of measure in the counter widget. It is interactive and it should be the same. By the way, I noticed another couple of things. I updated the review with these points |
| </FormGroup> : null} | ||
| </Form> | ||
|
|
||
| {formOptions.advancedOptions && data.type === "bar" || data.type === "line" ? |
Description
A few sentences describing the overall goals of the pull request' s commits.
Issues
Please check if the PR fulfills these requirements
What kind of change does this PR introduce? (check one with "x", remove the others)
What is the current behavior? (You can also link to an open issue here)
see 2654 for details
What is the new behavior?
added a collapse panel with advanced options, enables to 1.hide the grid. 2. hide y axis (shown b default) 3. add a y axis label
Does this PR introduce a breaking change? (check one with "x", remove the other)
If this PR contains a breaking change, please describe the impact and migration path for existing applications: ...
Other information: