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

Outside text relativebarmode confusing when bars disconnected #3116

Closed
eivindjahren opened this issue Oct 17, 2018 · 5 comments
Closed

Outside text relativebarmode confusing when bars disconnected #3116

eivindjahren opened this issue Oct 17, 2018 · 5 comments
Labels
bug something broken

Comments

@eivindjahren
Copy link
Contributor

eivindjahren commented Oct 17, 2018

With relative barmode and {textposition: 'outside'}, outside text will smartly position the text of the first bar as 'inside'. However, when there are two disconnected bars this results in the text of both bars being 'inside'. This is not a problem when barmode has the default value.

https://codepen.io/haskellelephant/pen/JmpJMZ

@alexcjohnson
Copy link
Collaborator

Interesting, there's at least one clear bug here, that the orange bar always gets its text pushed inside if it has a base, even if there are no other bars in that column at all (ie if you hide the blue one).

But the logic for the blue trace is a bit more complicated. I guess the simplest option would be if the trace stacked on it has a base at all, no matter the value, we don't automatically push text inside. Basically, once you've specified base (for the next trace) you're on your own with textposition and it's up to you to ensure it doesn't overlap another bar. We could try to figure out whether there's room outside a bar for text, based on the other bars around, but I suspect no matter what we'll make the wrong choice sometimes in some users eyes at least, so a better bet would be to make a clear statement:

If the trace above has a base -> no auto-inside text, you have to do it yourself.

Thanks @eivindjahren - another very clear and helpful report!

@eivindjahren
Copy link
Contributor Author

eivindjahren commented Oct 24, 2018

@alexcjohnson I have made a work in progress PR on my fork (first PR here so needed to conform to the contribution guidelines). It should implement your proposed solution. I still want to test it out a bit more, but hopefully this is in the right direction.

Another solution is to have 'outside' always mean 'outside', if you want the smart behavior you use 'auto'.

@alexcjohnson
Copy link
Collaborator

Great PR @eivindjahren - I'm always happy to see a one-line change in src backed up by 60 lines of tests!

Another solution is to have 'outside' always mean 'outside', if you want the smart behavior you use 'auto'.

Possible, though the existing 'auto' logic depends on the text size relative to the bar height, and ends up meaning "usually inside, but put it outside if we can and the bar is too small" - would be nice to clarify this behavior (for both 'auto' and 'outside') in the textposition description though!

@eivindjahren
Copy link
Contributor Author

@alexcjohnson I tried to summarize the behavior of outside in the textposition description, and adressed your comment about splitting the testdata trace. Not sure if the description is helpful though.

@alexcjohnson
Copy link
Collaborator

Closed by #3156 🎉 (correct me and reopen if I missed something)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug something broken
Projects
None yet
Development

No branches or pull requests

2 participants