rusackas commented on a change in pull request #9525: Migrating shared NVD3
controls to new module
URL:
https://github.com/apache/incubator-superset/pull/9525#discussion_r409991982
##########
File path: superset-frontend/src/explore/controlPanels/TimePivot.js
##########
@@ -17,47 +17,99 @@
* under the License.
*/
import { t } from '@superset-ui/translation';
-import { D3_TIME_FORMAT_OPTIONS } from '../controls';
+import { D3_FORMAT_OPTIONS } from '../controls';
+import {
+ lineInterpolation,
+ showLegend,
+ xAxisLabel,
+ bottomMargin,
+ xAxisFormat,
+ yLogScale,
+ yAxisBounds,
+ xAxisShowMinmax,
+ yAxisShowMinmax,
+ yAxisLabel,
+ leftMargin,
+} from './Shared_NVD3';
export default {
requiresTime: true,
controlPanelSections: [
{
label: t('Query'),
expanded: true,
- controlSetRows: [['metric'], ['adhoc_filters'], ['freq']],
+ controlSetRows: [
+ ['metric'],
+ ['adhoc_filters'],
+ [
+ {
+ name: 'freq',
+ config: {
+ type: 'SelectControl',
+ label: t('Frequency'),
+ default: 'W-MON',
+ freeForm: true,
+ clearable: false,
+ choices: [
+ ['AS', 'Year (freq=AS)'],
+ ['52W-MON', '52 weeks starting Monday (freq=52W-MON)'],
+ ['W-SUN', '1 week starting Sunday (freq=W-SUN)'],
+ ['W-MON', '1 week starting Monday (freq=W-MON)'],
+ ['D', 'Day (freq=D)'],
+ ['4W-MON', '4 weeks (freq=4W-MON)'],
+ ],
+ description: t(
+ `The periodicity over which to pivot time. Users can provide
+ "Pandas" offset alias.
+ Click on the info bubble for more details on accepted "freq"
expressions.`,
+ ),
+ tooltipOnClick: () => {
+ window.open(
+
'https://pandas.pydata.org/pandas-docs/stable/timeseries.html#offset-aliases',
+ );
+ },
+ },
+ },
+ ],
+ ],
},
{
label: t('Chart Options'),
expanded: true,
controlSetRows: [
- ['show_legend', 'line_interpolation'],
+ [showLegend, lineInterpolation],
['color_picker', null],
],
},
{
label: t('X Axis'),
expanded: true,
controlSetRows: [
- ['x_axis_label', 'bottom_margin'],
- ['x_axis_showminmax', 'x_axis_format'],
+ [xAxisLabel, bottomMargin],
+ [
+ xAxisShowMinmax,
+ {
Review comment:
Honestly, I just made the mistake because I was having a copy-paste party
late at night. I don't think I would have made a mistake if I were actually
trying to override a control with a specific intent.
My concern is that a function like this adds another onion layer for devs to
figure out. Figuring out that you need to pass in a complete config and a
partial config to this function (and why) seems like it might be just as
error-prone.
I think the spread syntax is straightforward enough for people to figure out
quite quickly. We could always re-enable `Object.assign` if that's any more
transparent in intent.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
