Component | Donut: Half Donut Label Placement

[curran]

This PR contains a simple solution to the half donut label placement problem that also could be used in the general case of donuts. Thank you @rokotyan for this suggestion!

If we will ultimately be doing text size estimation/measuring, then the changes in the other PR

• Component | Donut: Automatic label placement #8

may not make sense to add yet.

Proposed plan:

• Merge this PR to add the config options
• Later do a separate PR that adds a more complex and robust solution that involves measuring/estimating the size of text

[curran]

Thanks @rokotyan for the feedback! I made the change, then edited the Git history so there's only one commit now.

If this looks good, feel free to merge and publish. Thanks!

[curran]

@rokotyan It looks like somehow these changes made it into the 1.5.0-exaforce branch without this PR getting merged.

I made an additional change here.based on feedback from @jakubjosef , then squashed the commit as you mentioned I should always do, and I think now the git history is in a strange place.

I could do something like make a new PR with just the type change? It's pretty minor.

[curran]

Also curious if I should merge my own PRs on this repo or let you do it @rokotyan , since it's usually tied to releases. Please let me know your preference. Thanks!

[jakubjosef]

It looks like somehow these changes made it into the 1.5.0-exaforce without this PR getting merged.

Nikita said in that Slack threat he published it from this PR.

Just fix this PR and Nikita will publish new version from it.

For some reason he doesn't want to merge it...

[curran]

I mean in the commit history on the 1.5.0-exaforce branch, you can see that the earlier version of the commit for this PR made it into that branch somehow, regardless of the package publishing:

https://github.com/ExaForce/unovis/commits/1.5.0-exaforce/

I would have expected this PR to automatically get closed if the branch was merged, even if from CLI.

IMO there's a lot of overhead and confusion around "cleaning up" the commit history by squashing commits and force pushing branches. My preference would be to just let the commits stack up and leave the history intact, so that we can leverage all the various GitHub features like automatically merging PRs, and keeping the ability to view the before/after on specific commits, which are tied to the comment threads on the PRs. I think many of those features get broken when I do the force push on the branch, which is required to re-write the commit history with the really nice and clean commit messages.

[jakubjosef]

@curran It's easy - just maintain nice commit history on your own. Squash feature makes people lazy and just throwing random commits because "it would be eventually squashed". I'm sad we have this enforced for our main repo, but that's my personal opinion.

Also using force-push on feature branches is normal, I'm doing it multiple times a day - when you need to add something to existing commit you should use amend and force push - this way you can maintain clean history very easily.

[rokotyan]

@curran Let's discuss it on our sync on Friday. This puzzle has many pieces because we also want the same changes to be contributed to the main Unovis repo.

[rokotyan]

Currently I'm merging and releasing manually

[curran]

This puzzle has many pieces because we also want the same changes to be contributed to the main Unovis repo.

Exactly! This is the pattern we need to figure out.

[curran]

Current plan:

• Pull in the text measuring logic from https://github.com/ExaForce/unovis/pull/8/files into here
• Only trigger the automatic offsets if manual overrides not present
• Align label and sub-label in left and right cases
• Consider adding a new parameter like labelPadding for adding a bit of a gap with the automatic label placement

[curran]

Current status:

[curran]

Requesting re-review please @rokotyan . I think this could be ready to go. Thanks!

[rokotyan]

@curran The code looks elegant, I like it! I'm planning to run it tomorrow and if everything goes well, I'll release it

[curran]

Closing in favor of #11