From bf140243e51cb45a516a9fef77f93790ddb8389b Mon Sep 17 00:00:00 2001
From: Lukas Hermann <lukashermann28@gmail.com>
Date: Mon, 11 Mar 2024 14:37:32 -0700
Subject: [PATCH] feat: add support for signals in mark clip (#9282)

## PR Description

In Vega, [mark supports a signal for the `clip`
property](https://vega.github.io/vega/docs/marks/#mark-clipping). To
round out the Vega-Lite API, we should support it here as well,
especially since it is trivial to add.

## Checklist

- [x] This PR is atomic (i.e., it fixes one issue at a time).
- [x] The title is a concise [semantic commit
message](https://www.conventionalcommits.org/) (e.g. "fix: correctly
handle undefined properties").
- [x] `yarn test` runs successfully
- For new features:
  - [x] Has unit tests.
  - [x] Has documentation under `site/docs/` + examples.

Tips:

-
https://medium.com/@greenberg/writing-pull-requests-your-coworkers-might-enjoy-reading-9d0307e93da3
is a nice article about writing a nice PR.
- Use draft PR for work in progress PRs / when you want early feedback
(https://github.blog/2019-02-14-introducing-draft-pull-requests/).

---------

Co-authored-by: GitHub Actions Bot <vega-actions-bot@users.noreply.github.com>
---
 build/vega-lite-schema.json    | 22 +++++++++---
 src/compile/mark/mark.ts       |  2 +-
 src/mark.ts                    |  2 +-
 test/compile/mark/mark.test.ts | 62 ++++++++++++++++++++++++++++++++++
 4 files changed, 82 insertions(+), 6 deletions(-)

diff --git a/build/vega-lite-schema.json b/build/vega-lite-schema.json
index 6bc330e266..6f8de4b46c 100644
--- a/build/vega-lite-schema.json
+++ b/build/vega-lite-schema.json
@@ -16749,8 +16749,15 @@
           ]
         },
         "clip": {
-          "description": "Whether a mark be clipped to the enclosing group’s width and height.",
-          "type": "boolean"
+          "anyOf": [
+            {
+              "type": "boolean"
+            },
+            {
+              "$ref": "#/definitions/ExprRef"
+            }
+          ],
+          "description": "Whether a mark be clipped to the enclosing group’s width and height."
         },
         "color": {
           "anyOf": [
@@ -18300,8 +18307,15 @@
           ]
         },
         "clip": {
-          "description": "Whether a mark be clipped to the enclosing group’s width and height.",
-          "type": "boolean"
+          "anyOf": [
+            {
+              "type": "boolean"
+            },
+            {
+              "$ref": "#/definitions/ExprRef"
+            }
+          ],
+          "description": "Whether a mark be clipped to the enclosing group’s width and height."
         },
         "color": {
           "anyOf": [
diff --git a/src/compile/mark/mark.ts b/src/compile/mark/mark.ts
index cf821cb509..74e241b4ff 100644
--- a/src/compile/mark/mark.ts
+++ b/src/compile/mark/mark.ts
@@ -323,7 +323,7 @@ function getMarkGroup(model: UnitModel, opt: {fromPrefix: string} = {fromPrefix:
     {
       name: model.getName('marks'),
       type: markCompiler[mark].vgMark,
-      ...(clip ? {clip: true} : {}),
+      ...(clip ? {clip} : {}),
       ...(style ? {style} : {}),
       ...(key ? {key: key.field} : {}),
       ...(sort ? {sort} : {}),
diff --git a/src/mark.ts b/src/mark.ts
index 77bcd40dc5..a779ff0421 100644
--- a/src/mark.ts
+++ b/src/mark.ts
@@ -554,7 +554,7 @@ export interface MarkDefMixins<ES extends ExprRef | SignalRef> {
   /**
    * Whether a mark be clipped to the enclosing group’s width and height.
    */
-  clip?: boolean;
+  clip?: boolean | ES;
 
   // Offset properties should not be a part of config
 
diff --git a/test/compile/mark/mark.test.ts b/test/compile/mark/mark.test.ts
index 00815dedc2..5f8e7c3f38 100644
--- a/test/compile/mark/mark.test.ts
+++ b/test/compile/mark/mark.test.ts
@@ -514,4 +514,66 @@ describe('Mark', () => {
       expect(mark[0].clip).toBe(true);
     });
   });
+
+  describe('signal clipping', () => {
+    it('should pass clip as a signal if clip is an expression', () => {
+      const model = parseUnitModelWithScaleAndLayoutSize({
+        mark: {
+          type: 'bar',
+          clip: {expr: "datum['foo'] > 10"}
+        },
+        encoding: {
+          x: {type: 'quantitative', field: 'foo'}
+        }
+      });
+
+      const markGroup = parseMarkGroups(model);
+      expect(markGroup[0].clip).toEqual({signal: "datum['foo'] > 10"});
+    });
+
+    it('should pass clip as a signal if clip is a signal', () => {
+      const model = parseUnitModelWithScaleAndLayoutSize({
+        mark: {
+          type: 'bar',
+          clip: {signal: "datum['foo'] > 10"}
+        },
+        encoding: {
+          x: {type: 'quantitative', field: 'foo'}
+        }
+      });
+
+      const markGroup = parseMarkGroups(model);
+      expect(markGroup[0].clip).toEqual({signal: "datum['foo'] > 10"});
+    });
+
+    it('should pass clip as a boolean if clip is true', () => {
+      const model = parseUnitModelWithScaleAndLayoutSize({
+        mark: {
+          type: 'bar',
+          clip: true
+        },
+        encoding: {
+          x: {type: 'quantitative', field: 'foo'}
+        }
+      });
+
+      const markGroup = parseMarkGroups(model);
+      expect(markGroup[0].clip).toBe(true);
+    });
+
+    it('should not have clip defined if clip is false', () => {
+      const model = parseUnitModelWithScaleAndLayoutSize({
+        mark: {
+          type: 'bar',
+          clip: false
+        },
+        encoding: {
+          x: {type: 'quantitative', field: 'foo'}
+        }
+      });
+
+      const markGroup = parseMarkGroups(model);
+      expect(markGroup[0].clip).toBeUndefined();
+    });
+  });
 });