Skip to content
This repository has been archived by the owner on Jul 29, 2019. It is now read-only.

Fix Vertical visibility for all item types #2143

Merged
merged 14 commits into from
Oct 17, 2016

Conversation

yotamberk
Copy link
Contributor

the calculation of every single item's visibility was fixed according to its type.

Vertical visibility is determined only by if the group itself is visible. Only groups that are visible in the timeline will expose there itemSets, then tested for horizontal visibility (for each of the item in these items is in range according to the item's isVisible function - differs by item type) and only then rendered to the DOM.

Every item has an isVisible function that checks if the item is in range. This calculation is different from item type to item type (ok... it is similar to ranged items and background items), and should not be defaulted in the Item class as mentioned in a TODO - hence I removed the comment.

The calculation for the msPerPixel is a simple calculation to insure the true width of box and point items is correct. Because these item types don't have an end time, I use the width of the item + it's start time to simulate it's end point with the widthInMs and subtract/add it according to align option.

@mojoaxel
Copy link
Member

@yotamberk Can you recommend an example to test this!?

@yotamberk
Copy link
Contributor Author

@mojoaxel It's a little bit hard to see because up until now the items would be visible horizontally even when not in range. I used debugging tools and the devtools to fix this. I can try to ad some kind of example. Give me a few hours =)

@mojoaxel
Copy link
Member

@yotamberk No I thinks that's ok. I ecpect you know what you are doing :-)

Copy link
Member

@mojoaxel mojoaxel left a comment

Choose a reason for hiding this comment

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

Code looks good. Could not proper test it, but makes sense!

@mojoaxel mojoaxel merged commit 932dfb1 into almende:develop Oct 17, 2016
@mojoaxel
Copy link
Member

@yotamberk Thanks!

@yotamberk
Copy link
Contributor Author

Thanks again =)

@yotamberk yotamberk deleted the vertical-visibility branch October 28, 2016 09:09
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants