Skip to content

fix(legend): add third mode, discrete, to LegendType#3957

Merged
hydrosquall merged 2 commits intomainfrom
cameron.yick/update-discrete-color-legend-type
Sep 10, 2024
Merged

fix(legend): add third mode, discrete, to LegendType#3957
hydrosquall merged 2 commits intomainfrom
cameron.yick/update-discrete-color-legend-type

Conversation

@hydrosquall
Copy link
Copy Markdown
Member

@hydrosquall hydrosquall commented Aug 2, 2024 •
edited
Loading

Motivation

Changes

  • Adds a type to align with the runtime behavior, + docstring update

vega/packages/vega-parser/src/parsers/legend.js

Lines 35 to 36 in ddd2bbf

// resolve legend type (symbol, gradient, or discrete gradient)
type = legendType(spec, scope.scaleType(scale));

  • (Checked w/ @jheer to confirm if this is the intended behavior based on the commit that introduced this, ddd2bbf

Notes

  • Broader question: would we be interested to have portions of the vega sub-packages converted to TS so we don't have to sync .d.ts files with JS source code as a separate step?

@hydrosquall hydrosquall self-assigned this Aug 2, 2024
@cloudflare-workers-and-pages
Copy link
Copy Markdown

cloudflare-workers-and-pages bot commented Aug 2, 2024 •
edited
Loading

Deploying vega with  Cloudflare Pages  Cloudflare Pages

Latest commit: 9736917
Status: ✅  Deploy successful!
Preview URL: https://a75619db.vega-628.pages.dev
Branch Preview URL: https://cameron-yick-update-discrete.vega-628.pages.dev

View logs

@hydrosquall hydrosquall marked this pull request as ready for review August 2, 2024 15:44
@hydrosquall hydrosquall requested a review from a team as a code owner August 2, 2024 15:44
@hydrosquall hydrosquall requested a review from jheer August 2, 2024 15:45
@hydrosquall hydrosquall requested a review from lsh August 18, 2024 18:17
@domoritz
Copy link
Copy Markdown
Member

domoritz commented Sep 6, 2024

Broader question: would we be interested to have portions of the vega sub-packages converted to TS so we don't have to sync .d.ts files with JS source code as a separate step?

Yes, if we can figure out a way that doesn't degrade the developer experience. So for example if I change code in a linked Vega package, the Vega editor (in local dev mode) should update.

domoritz
domoritz requested changes Sep 6, 2024
View reviewed changes
Copy link
Copy Markdown
Member

@domoritz domoritz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please update the docs at https://vega.github.io/vega/docs/legends/ as well.

@hydrosquall hydrosquall mentioned this pull request Sep 8, 2024
Open
@hydrosquall
Copy link
Copy Markdown
Member Author

hydrosquall commented Sep 8, 2024 •
edited
Loading

Yes, if we can figure out a way that doesn't degrade the developer experience. So for example if I change code in a linked Vega package, the Vega editor (in local dev mode) should update.

Got it! I split this out to a separate issue. #3971

Please update the docs

A separate followup would be to generate sections of the docs files (like this props table) from the JSDoc comments in the source code. For the time being, I made a manual update 9736917 , thanks for the flagging this one.

@hydrosquall hydrosquall requested a review from domoritz September 8, 2024 17:42
@hydrosquall hydrosquall merged commit c5c084b into main Sep 10, 2024
@hydrosquall hydrosquall deleted the cameron.yick/update-discrete-color-legend-type branch September 10, 2024 13:26
@hydrosquall hydrosquall mentioned this pull request Jan 11, 2025
Draft
@lsh lsh mentioned this pull request Jan 24, 2025
Merged
lsh added a commit that referenced this pull request Jan 24, 2025
@lsh
chore: v5.31.0 (#4003)
0e1eb8b 
changes since v5.30.0

**vega-utils**
* use `Object.hasOwn` instead of `Object.prototype.hasOwnProperty` (via
#3951). (Thanks @domoritz!)

**vega-parser**
* Add discrete legend type (via #3957). (Thanks @hydrosquall!)

**vega-functions**
* Add sort function to vega-functions (and vega-interpreter) (via
#3973). (Thanks @hydrosquall!)

**vega-selections**
* Add field predicate types to selectionTest (via #3675). (Thanks
@jonathanzong!)

**monorepo**
* Replace flights-5k.json external reference (via #3999). (Thanks
@dwootton!)

**docs**
* Update packed bubble example (via #3955). (Thanks @PBI-David!)
* Correct typo in production rules documentation (via #3958). (Thanks
@shanebruggeman!)
* Update README.md to fix broken link to current roadmap (via #3979).
(Thanks @cahogan & @joelostblom!)

---------

Signed-off-by: Lukas Hermann <1734032+lsh@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@domoritz domoritz domoritz approved these changes

@jheer jheer Awaiting requested review from jheer

@lsh lsh Awaiting requested review from lsh

Assignees

@hydrosquall hydrosquall

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

fix: Update LegendType typescript definition to support 3rd (discrete) type

2 participants

@hydrosquall @domoritz