Skip to content

ignore empty facets, take 2 - #101

Closed
Fil wants to merge 2 commits into
mainfrom
empty-facets-2
Closed

Fil wants to merge 2 commits into
mainfrom
empty-facets-2

Conversation

@Fil

@Fil Fil commented Jan 14, 2021 •

Copy link
Copy Markdown
Contributor

@Fil
Fil requested a review from mbostock January 14, 2021 17:17
@Fil

Fil commented Jan 14, 2021

Copy link
Copy Markdown
Contributor Author

Two things worry me a bit in this PR:

  • the single path means that we can't select a facet and move it around (in Illustrator); it seems to me that this would be better in the other approach (faceted grid #98)
  • the code is a bit weird: we send a filtered domain of fx in index, or undefined if there is only 1 facet axis, but when index is undefined we used fy.domain

@mbostock mbostock added this to the Friends Preview milestone Feb 24, 2021
@mbostock mbostock mentioned this pull request Feb 24, 2021
@mbostock mbostock removed this from the Friends Preview milestone Mar 10, 2021
@Fil Fil mentioned this pull request Mar 27, 2021
@Fil
Fil force-pushed the empty-facets-2 branch from 2ff3c4c to 3dbebc1 Compare April 2, 2021 12:27
@Fil

Fil commented Apr 2, 2021

Copy link
Copy Markdown
Contributor Author

I've rebased this PR. Moving to draft while I work on #277.


I've also tried to add axes on (x,y) facets that have no left/bottom neighbor. It sort of works

it works

but it would create two new issues:

  1. Where should we put the axis label?

position1

position2

  1. The axes tend to collide

collision

@Fil
Fil marked this pull request as draft April 2, 2021 12:37
@Fil
Fil marked this pull request as ready for review April 2, 2021 14:14
@Fil Fil mentioned this pull request Apr 19, 2021
3 tasks
@Fil

Fil commented Apr 19, 2021

Copy link
Copy Markdown
Contributor Author

superseded by #332

@Fil Fil closed this Apr 19, 2021
@Fil
Fil deleted the empty-facets-2 branch April 19, 2021 15:22
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.

Hide empty facets?

2 participants