Skip to content
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

Legend supports opacity channel #1375

Merged
merged 4 commits into from
Jul 9, 2016
Merged

Legend supports opacity channel #1375

merged 4 commits into from
Jul 9, 2016

Conversation

YuhanLu
Copy link
Contributor

@YuhanLu YuhanLu commented May 19, 2016

Fix #1349
Now legend supports to show opacity channel

{
  "data": {"url": "data/seattle-weather.csv", "formatType": "csv"},
  "mark": "bar",
  "config": {
    "mark": { "opacity": 0.5 }
  },
  "encoding": {
    "x": {
      "field": "date",
      "type": "temporal",
      "timeUnit": "month"
    },
    "y": {
      "field": "*",
      "type": "quantitative",
      "aggregate": "count"
    },
    "opacity": {
      "field": "*",
      "type": "quantitative",
      "aggregate": "count"
    }
  }
}

@@ -158,6 +160,10 @@ export namespace properties {
symbols.strokeWidth = { value: 0 };
}

if (channel === OPACITY) {
delete symbols.opacity;
Copy link
Member

Choose a reason for hiding this comment

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

// Avoid override default mapping for opacity channel

Copy link
Contributor Author

@YuhanLu YuhanLu May 19, 2016

Choose a reason for hiding this comment

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

Do you have any suggestion how should we avoid this? Since symbols are initialized in applyMarkConfig() (line 157), I think it's better to do any modification in legend.ts instead of other places.

Copy link
Member

Choose a reason for hiding this comment

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

I think this is fine. I mean add a comment

// Avoid override default mapping for opacity channel

Sorry for not being clear.

@kanitw kanitw added the Blocked 🕐 For issues that are blocked by other issues label May 23, 2016
@kanitw
Copy link
Member

kanitw commented Jun 19, 2016

Still missing test and I guess we need to wait for a new Vega release, which is coming soon?

# Conflicts:
#	src/compile/legend.ts
@kanitw kanitw removed the Blocked 🕐 For issues that are blocked by other issues label Jul 9, 2016
@kanitw kanitw merged commit 8f5ed7c into master Jul 9, 2016
@kanitw kanitw deleted the zl/legend-opacity branch July 9, 2016 18:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants