Skip to content

DM-7966: Default table sorting in LC viewer #251

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

Merged
merged 1 commit into from
Dec 16, 2016
Merged

Conversation

loitly
Copy link
Contributor

@loitly loitly commented Dec 14, 2016

https://jira.lsstcorp.org/browse/DM-7966

  • set default sorting on raw, phase, and peak tables
    also:
  • allow for raw table upload only, but added option to specify time and flux column
  • code cleanup
  • fix table index out-of-bound when navigate via arrow keys
  • change popup phase folded values from 3 decimal places to 8 to match the server-side version

Use /firefly/lc.html to test.

 - set default sorting on raw, phase, and peak tables
 also:
 - allow for raw table upload only, but added option to specify time and flux column
 - code cleanup
 - fix table index out-of-bound when navigate via arrow keys
 - change popup phase folded values from 3 decimal places to 8 to match the server-side version
@ejoliet
Copy link
Contributor

ejoliet commented Dec 16, 2016

Great! Sorted tables works as expected.

Minor thing that worth be noticing but I don't think it is part of the ticket, once i choose a flux column, i would expect to see the same in the other panels (period finding, phase folded,etc.). What do you think? Maybe this should be part of the layout change and how we use the same column name elsewhere.

2 minor issues though:

  1. When i close one table, the other one is unresponsive.
  2. if i upload another raw lc, i would expect the rest of the tables to be cleared out because they are not related but i don't know if this is a right workflow. Could be considered.

Maybe would be good to check at least the first issue before merging merge.

@loitly
Copy link
Contributor Author

loitly commented Dec 16, 2016

The issues you raised are existing issues that should be resolved in other pull requests especially since we have open tickets to revamp the flow and layout of this app. I recommend merging this pull request as is, and address these issues in the other tickets. What do you think?

@loitly loitly merged commit 304c80f into dev Dec 16, 2016
@loitly loitly deleted the DM-7966_lc_default_sort branch December 16, 2016 22:43
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

Successfully merging this pull request may close these issues.

2 participants