-
Notifications
You must be signed in to change notification settings - Fork 0
Adding historical data table view for all types of subjecs #662
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
Conversation
…interval issues with 'pingSocket'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think Patsy did a great job on the UI side of this PR. I just added some questions related to the code.
…popup renderer. code and test refactors to accommodate.
Hey @JoshuaVulcan did you miss pushing changes? I see a few comments with no response or update 🤔 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is ready overall, nothing to block it. Still I added a couple comments 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the historical data button looks bigger than others:
some paddings and message image is a little too big
but the weird thing is I only see the heatmap button being cut in the environment but not in my local, I think the environment could have some missing updates:
👉 I only consider as a blocker the size of the track control in the patrols list
@amicavi Working on the other funk, but the overflow issue with the track controls in the list has been fixed 😎 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Hey! @JoshuaVulcan @amicavi helped me on testing this one, we were able to see the historical data for both Stationary and Non-Stationary subjects. The only thing that seems to be wrong is the icon for Historic Data ( It has the same icon as |
@Alcoto95 woops! Good catch, I'll get that updated |
@Alcoto95 @YazzMagana this has been updated, and those updates were related to minor aesthetic fixes. Can we consider this merge-able so it can go out with the sprint? |
https://allenai.atlassian.net/browse/DAS-8038
https://das-8038.pamdas.org
Now that the "historical data" button is available for all subject types, this neatly belongs in the SubjectControls component. As such, lots of code de-duplication and refactoring was necessary.