Open acircleda opened 3 years ago
thanks for that, @acircleda, it's been a while since either of us worked on this project, it's good to see your recent contribution and you put a lot of work into it, so thank you! 👍 Here are my thoughts following the initial review:
This is great progress but I don't think this PR is ready to be merged yet. Some things I noticed:
absolute-panel
branch so maybe we could reuse the code from there? visualizations.R
shouldn't be in the root directory, perhaps we could move to somewhere like www/scripts
?visualizations.R
on my machine. Some culprits are easy to resolve, e.g. loading ggplot2
, skipping execution of theme_set(theme(text = element_text(family = "Open Sans")))
, defining parts
object, etc. but some are not obvious, e.g. running the graphic
code beyond ggtitile()
part returns:
Error in grid.Call(C_textBounds, as.graphicsAnnot(x$label), x$x, x$y, : polygon edge not found
There's a lot going on! So I wonder if in the first instance we could leave the changes for of search engine and new equivalents in this PR and work on the remaining parts in smaller steps/multiple PRs? It's easier this way to make iterative improvements. Looking forward to your comments 👍
Hi Kasia,
I agree there is a lot going on. We can actually delete visualizations.R. It was just a sandbox to test out the visualizations. Let me focus on the search engine and equivalents and I'll push that. Then we can get to the other parts.
The "Add Leg" feature was inspired by the ICAO calculator and I almost have it working, too, but I will save that for later.
Some airports do not have IATA codes (they begin with \N) but they do have IATA codes. We may have to address this. Search is better but not optimal. Searching for the airport PIE does not immediately find the match. I may address this with the dqshiny author.
I made a few changes to the app and emission results. I also added a methodology section, but am having some trouble with the layout.