Page MenuHomePhabricator

use blue print
ClosedPublic

Authored by brooke on Aug 14 2019, 7:57 PM.

Details

Summary

Sorry this revision includes 3 items:

  1. Implement blue print
  2. Add class to simui_table_view.py for css update
  3. Add a class for numeric inputs (simui_input_view.py). This might need to be expanded in the future but I only need to use min/max function for numeric inputs for now.

I am not sure this should be a class for numeric inputs only or all inputs?

Thoughts? @chris

Test Plan

code review

Diff Detail

Repository
rSIM Simplex
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

brooke requested review of this revision.Aug 14 2019, 7:57 PM
brooke created this revision.
brooke edited the summary of this revision. (Show Details)Aug 14 2019, 8:05 PM
chris added a comment.Aug 18 2019, 1:39 PM

So I think that for now we might want to keep the view and route folders separate, where view is more or less "How do we bring together all of the UI components on the page" and route is "What Python gets run when we go to a URL". It looks like you have all of the blueprint routing in view currently.

Down the road, it might make sense to combine these two, but I'd rather keep them separate for now until we get a better idea of how each of those pieces will mature.

I've got a couple UI nitpicks over in

{M1}

if you don't mind looking through those.

Otherwise, just the usual "Fix the lint issues" type of stuff.

simplex/templates/manage/data.html
30

I'd still like you to get away from using so many <br /> tags. Like, they're useful, but I think we can accomplish the same thing more reliably using better CSS styles.

brooke added a comment.EditedAug 18 2019, 1:45 PM

I will move things from view to route.
I will update the UI items including

  • style header
  • style button
  • combine tables
  • replace index with headers
simplex/templates/manage/data.html
30

Agree. I will remove them

brooke updated this revision to Diff 81.Aug 18 2019, 3:16 PM
  • address code review issues
brooke updated this revision to Diff 82.Aug 18 2019, 4:15 PM
  • fix lint issues
chris added inline comments.Aug 18 2019, 4:26 PM
simplex/application/data/data_management.py
41

Which is canonical: .T or .transpose()?

45

You verified that column ordering will always be correct and identical between the two?

47

Why are headers getting duplicated in the concatination?

simplex/route/data_view.py
11

Should be two blank lines between the last from ... import ... and this line.

15–16

Lines 14/15 can be removed

brooke updated this revision to Diff 84.Aug 18 2019, 4:50 PM
  • address code review issues
  • fix lint issues
  • make it work after rebasing
chris accepted this revision.Aug 18 2019, 4:51 PM
This revision is now accepted and ready to land.Aug 18 2019, 4:51 PM
This revision was automatically updated to reflect the committed changes.