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

new disclosure behavior breaking fill area drop down #5743

Closed
maxgrossman opened this issue Jan 21, 2019 · 6 comments
Closed

new disclosure behavior breaking fill area drop down #5743

maxgrossman opened this issue Jan 21, 2019 · 6 comments
Assignees
Labels
bug-release-blocker An important bug - let's get this fixed in the next release!

Comments

@maxgrossman
Copy link
Collaborator

maxgrossman commented Jan 21, 2019

I noticed that the fill area drop down intermittently does not open anything when I click it.

I did some investigation and the issue seems to have to do with a recent change to the uiDisclosure class.

What seems to be happening is that the above line is what is responsible for responsible for calling the renderFillList function that turns _fillList from null to a selection of the area fill unordered list.

In the cases when this function is not called, the drawListItems function is given an empty selection and so it never executes its intended rendering.

Would adding some sort of additional param to the disclosure class to force that the disclosure to be called in certain cases achieve fixing this bug and keep @bhousel's recent change?

I'm thinking it could make that disclosure line something like...

if (_expanded || forceExpand) {
    _wrap.call(_content)
}

where force expand is a new parameter to the class and only in select cases like the the one above do we ever provided the value true.

@maxgrossman
Copy link
Collaborator Author

here's a gif
map_data-dropdown

I think the reason the second time it works is because of what we set _expanded to here

I guess on reload _preferences has a value so _expanded is set to true

@maxgrossman
Copy link
Collaborator Author

well darn, as one reads code they see that the feature I suggested is already implemented!

...so i guess the real question is, can we add true for default expanded...

@quincylvania quincylvania added the bug-release-blocker An important bug - let's get this fixed in the next release! label Jan 21, 2019
@quincylvania quincylvania self-assigned this Jan 21, 2019
@quincylvania quincylvania added the wip Work in progress label Jan 21, 2019
@quincylvania quincylvania removed the wip Work in progress label Jan 21, 2019
@quincylvania
Copy link
Collaborator

@maxgrossman Thanks for finding this! I fixed it by calling the appropriate update() code for each section after rendering.

This gives me another idea: we could avoid running update on hidden sections since they will be reloaded anyway when opened.

@maxgrossman
Copy link
Collaborator Author

@quincylvania - you rule! such a quick fix.

to your point - sounds like a good idea.
I did notice when I was debugging that the update does get called / cause the same drawFillAreas code to get executed quite a few times, which if avoidable would be good I think

@quincylvania
Copy link
Collaborator

@maxgrossman Thanks! I implemented the check for most of the sections in 1323ec3.

@maxgrossman
Copy link
Collaborator Author

🚤

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug-release-blocker An important bug - let's get this fixed in the next release!
Projects
None yet
Development

No branches or pull requests

2 participants