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

Container annotation and marker index #1061

Open
Integral opened this issue Jun 19, 2017 · 8 comments
Open

Container annotation and marker index #1061

Integral opened this issue Jun 19, 2017 · 8 comments
Assignees

Comments

@Integral
Copy link
Member

@oliver---- just a noisy reminder about new container annotation problems.
After new marker manager API was introduced marker index is no longer inherited from document index and we have no container annotation markers inside.
You proposed to deal with container annotations in it's own manager and package.

@obuchtala
Copy link
Member

obuchtala commented Jun 19, 2017 via email

@Integral
Copy link
Member Author

Integral commented Aug 1, 2017

Just FYI there is #1095 which is it.
However I decided not to move models and index into this package due to number of issues. For instance it is hard to deal with them when it's comes to transaction documents which is cloning document before manager gets connected.
Basically it's your February work plus manager and without topsy-turvy'ish project ;)

@Integral
Copy link
Member Author

Integral commented Aug 1, 2017

I found this implementation of manager a bit naive and with large set of container annotations everything becoming slow. Instead of rerendering whole set of fragment markers we should rerender only those who related to changed text nodes. Working on this.

@obuchtala
Copy link
Member

I absolutely agree.

@Integral
Copy link
Member Author

Integral commented Aug 2, 2017

There are still cases which is not handled properly. For instance container annotation didn't got deleted when you use container selection with the annotation inside. Probably we should improve test suite...

@Integral
Copy link
Member Author

Integral commented Aug 5, 2017

@oliver---- would you please be so kind to review the PR? It is still have at least one bug which I'm aware about, but i'm still didn't trace all the circumstances. Your general feedback and code quality review will be very much helpful. There are not too much changes compared to develop brunch. Please!

Sent from my Xiaomi Redmi 3S using FastHub

@Integral
Copy link
Member Author

Integral commented Aug 6, 2017

Ok, that bug also fixed now...

@obuchtala
Copy link
Member

Will do

@obuchtala obuchtala self-assigned this Aug 8, 2017
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

No branches or pull requests

2 participants