-
Notifications
You must be signed in to change notification settings - Fork 626
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: layer with shared encoding ignore repeater #9246
base: main
Are you sure you want to change the base?
Changes from 8 commits
e64fe8b
7cbd218
9800e97
74fb9c1
5a46f11
6a02b08
5128a05
1421933
9d0c745
4f30bf2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -68,7 +68,6 @@ export class CoreNormalizer extends SpecMapper<NormalizerParams, FacetedUnitSpec | |
...(spec.name ? {name: [params.repeaterPrefix, spec.name].filter(n => n).join('_')} : {}), | ||
...(encoding ? {encoding} : {}) | ||
}; | ||
|
||
if (parentEncoding || parentProjection) { | ||
return this.mapUnitWithParentEncodingOrProjection(specWithReplacedEncoding, params); | ||
} | ||
|
@@ -212,6 +211,7 @@ export class CoreNormalizer extends SpecMapper<NormalizerParams, FacetedUnitSpec | |
|
||
return super.mapFacet(spec, params); | ||
} | ||
// | ||
|
||
private mapUnitWithParentEncodingOrProjection( | ||
spec: FacetedUnitSpec<Field>, | ||
|
@@ -224,14 +224,21 @@ export class CoreNormalizer extends SpecMapper<NormalizerParams, FacetedUnitSpec | |
parentEncoding, | ||
encoding: replaceRepeaterInEncoding(encoding, params.repeater) | ||
}); | ||
|
||
return this.mapUnit( | ||
{ | ||
...spec, | ||
...(mergedProjection ? {projection: mergedProjection} : {}), | ||
...(mergedEncoding ? {encoding: mergedEncoding} : {}) | ||
}, | ||
{config} | ||
emptyFlag(encoding) && !emptyFlag(params.repeater) | ||
? { | ||
...params, | ||
config: config, | ||
repeater: params.repeater | ||
} | ||
: { | ||
config | ||
} | ||
); | ||
} | ||
|
||
|
@@ -398,3 +405,10 @@ function mergeProjection<ES extends ExprRef | SignalRef>(opt: { | |
} | ||
return projection ?? parentProjection; | ||
} | ||
|
||
function emptyFlag(obj: any) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was thinking this should be it utils. We already have There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Suggested code above. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. good to know that! |
||
if (obj == null) { | ||
return true; | ||
} | ||
return Object.keys(obj).length === 0; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.