Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Revise operation in rangebreaks #4660

Merged
merged 7 commits into from
Mar 18, 2020
Merged

Conversation

archmoj
Copy link
Contributor

@archmoj archmoj commented Mar 18, 2020

Follow up of #4614, and in regards with #4655 (comment) and #4655 (comment)
commits in this PR are steps to change default operation to [) and make it internal.

@plotly/plotly_js

@archmoj archmoj added this to the v1.53.0 milestone Mar 18, 2020
@archmoj archmoj requested a review from alexcjohnson March 18, 2020 03:58
@archmoj archmoj added the bug something broken label Mar 18, 2020

if(onOpenBound && op0 === '(' && v === b0) return BADNUM;
onOpenBound = op1 === ')' && v === b1;
if(onOpenBound && v === b0) return BADNUM;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Looks like onOpenBound can just be 🔪 at this point?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good eye. Done in c4dde70.

},
{
"pattern": "hour",
"bounds": [ 16, 8 ],
"operation": "()"
"bounds": [ 16.001, 8 ]
Copy link
Collaborator

Choose a reason for hiding this comment

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

🙃

Copy link
Collaborator

@alexcjohnson alexcjohnson left a comment

Choose a reason for hiding this comment

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

Fantastic! Just one small comment https://github.com/plotly/plotly.js/pull/4660/files?short_path=6ccd23f#r394320834 and this is ready to 💃

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

Successfully merging this pull request may close these issues.

2 participants