Outdated
added a separate refreshQuery state parameter to prevent to too many queries. Outside of SkyView structure, because updating that structure inside the component caused an infinite loop somehow. (Aladin still renders too often, but less severely so)
Also added the timestamp again, but hopefully not breaking the release now.
And moved the buttons in the Results box to the left instead of the right (but layout still looks misaligned)
Fanna Lautenbach (71cdadd1) at 31 Aug 07:52
Release 0.6.0
Fanna Lautenbach (ecb07d99) at 31 Aug 07:52
Release 0.6.0
Fanna Lautenbach (f4c64d90) at 31 Aug 07:50
_Note: _after checking the renders with console.log statements everywhere, I can now conclude that the controllers actually work great and only trigger re-renders on the correct state changes with the changes here. Since there are dependent fields (for example: frequency range should change after deselecting an archive) within the SkyViewController, per request of our stakeholders, multiple re-renders on the menu component cannot be bypasses
Note2: don't forget an npm install if you want to run tests as some libraries have been added for testing purposes
Fanna Lautenbach (9983a814) at 31 Aug 07:50
Merge branch 'run-query-with-refetch' into 'master'
... and 1 more commit
_Note: _after checking the renders with console.log statements everywhere, I can now conclude that the controllers actually work great and only trigger re-renders on the correct state changes with the changes here. Since there are dependent fields (for example: frequency range should change after deselecting an archive) within the SkyViewController, per request of our stakeholders, multiple re-renders on the menu component cannot be bypasses
Note2: don't forget an npm install if you want to run tests as some libraries have been added for testing purposes
_Note: _after checking the renders with console.log statements everywhere, I can now conclude that the controllers actually work great and only trigger re-renders on the correct state changes with the changes here. Since there are dependent fields (for example: frequency range should change after deselecting an archive) within the SkyViewController, per request of our stakeholders, multiple re-renders on the menu component cannot be bypasses
Note2: don't forget an npm install if you want to run tests as some libraries have been added for testing purposes
I tried that and that is unfortunately not the case since the data array changes all the time (just now what's in it). It needs to do an additional check, via a memo, to see if data has changed. As you pointed out, this works but I would like expert advise though where I should put it; by you that is;).
Calling the function via setting a toggle with a useEffect was what it did before. To remove the dependency on some field (the toggle that triggers the fetching) I just call the function which makes it a lot clearer.
@kliffen With regard to the query; it is based upon moving/zooming. It shouldn't query on every move/zoom but only when the change is large enough. So it does take into account what is currently visible
Yes!
I agree. But since the back-end is indeed a problem and should be resolved in the future with super fast queries, I don't want to add something like that now
@kliffen I made a ticket for implementing the catalog element so that should be taken into account
@vermaas It should be switched of because we have all the UI elements in the menu. This is also what Marco and Manu said in one of our feedback sessions; they were confused where to click (Menu of within Aladin Light)
I fully don't understand what I see here either, but it could be that those parameters are in the 'skyView' parameter? And the RA,dec are set in another function in this class in a similar way as fov.
I originally wrote this function to do 2 things:
The way I wrote it is that only 'state' was set, and no fetch functions were called. (They call themselves through useEffect) That way you keep the dependencies a bit clearer and you can easier set the state from other places in the application without running into dependency trouble.
I don't think it is totally wrong to call 'runQuery', but I find it a bit confusing, especially without seeing what parameters it takes.
I agree with that.
I actually tried to keep track and document the flow of state and dependencies in ADEX, but it changed so much that I lost track. And then I cannot see where potential problems arise anymore either. https://drive.google.com/file/d/1hpwyImdLpKbOYmOFV2L6uu2LW1oN36kg/view?usp=sharing
It would indeed be good to have this documented and kept up-to-date (it doesn't perse have to be in the form that I use, as long as it gives enough information and is kept up-to-date.
Indeed, no more paradigm shifts or new libraries please. I am looking to simplify matters to get a higher velocity for development (or, the KISS pattern, keep it simple stupid, because poor Nico is planning to get back to ADEX programming and he has to understand this weird javascript stuff also.... that is more important than involved solutions that leave me flabbergasted)
I'm planning to phase out the queries to ESAP anyway, so it may all work a bit differently in the future (if I get my plan past the powers that be).
The showLayersControl becomes a bit of a wellus/nietus thing. You switched it off, I switched it back on, now you switch it off again. I prefer to have it switched on, because that gives users a lot more surveys to choose from than the few that we offer.
@kliffen ,That is something that I would like to implement, but I need some more explanation about what you exactly did, because I cannot understand the typescript javascript dialect that you used. So if you can provide an example in the javascript style that I used then that would be very helpful. (or just explain it in a way that I understand ;-) ).