Closed vorugantia closed 7 years ago
Use spacing so that the values are all listed at the same spot vertically.
Is it really better readable if vertically aligned? Or is it about the same or even better to just do no vertical alignment at all?
Before spending much time on that, I would suggest you do a few values in the two styles and show a screenshot.
The app doesn't work for me. In the console I see failed GET requests e.g. to http://localhost:4200/data/cat/3fgl/cat.json .
I did generate the files with the Python script, and it put them here: src/app/data/cat/3fgl/cat.json
Did you forget to commit some change? Can you come by my office and we have a look / discuss?
I'm not sure how to best surface missing data in the UI. Yes, what you have now with "Position error: deg" is no good. Two options which I find roughly equally good:
I think in any case, this requires an "if" statement either in the HTML template or in your TS code on the catalog source class. Another option would be to do this "transform to UI presentation" in Python, i.e. directly generate the strings you then surface in the app there. I think this isn't a great solution though, and would suggest to just transmit the numbers / missing data via None in the JSON files, and do the UI presentation in TS or HTML.
Ok, I've made all of the changes you suggested. You mentioned a few minor changes that I can add after tests etc. - I will keep those in mind.
So far, I have done the following in this PR:
Fetch individual source data from JSON source files. Access this data when you click on the specified source in
cat-search.component.ts
BASIC source detail view layout for TeV sources only (I still need to do the others).
@cdeil - I wanted to open the PR now so that you could have a look and tell me what to edit now. Run the
make.py
commands, callnpm start
and have a look at a TeV detail page. So far, I think I should fix the following:null
shows up as nothing in the HTML, but the units still show (i.e. Vela X printsPosition error: deg
). How should I fix this? Perhaps from the Python side I can just change allNaN
values to"null"
, instead ofnull
?Let me know if there is anything else I should fix.