Fix #1473: Add navigation and create feeds page #1475
Conversation
| ) | ||
|
|
||
|
|
||
| def layout(app): |
There was a problem hiding this comment.
I would suggest converting all of these to a class, now. Since we evolved the knowledge required by each component, I would suggest creating Page/AppPage objects which take in app in the constructor. Functions that use knowledge from app or any other common structures can therefore be independent and parameter passing reduced. We can define a light interface using abc that requires all pages to have a layout() function. And maybe a main container and a nav which knows its own location.
In time, the procedural approach will get too complicated to manage so it's better to do this now.
|
|
||
| selected_predictoors = list(range(len(app.favourite_addresses))) | ||
|
|
||
| if app.favourite_addresses: |
There was a problem hiding this comment.
I know this is my work, but seeing it moved helped me realise we can simplify the ifs a bit. If we move app as a class attribute and it's easier to separate, let's retrieve the selected feeds in a separate function and keep the functional part away from the layout part. i.e. have a function "get_feeds_for_predictoors" instead of this block here (58-71)
| return html.Div( | ||
| [ | ||
| html.Div( | ||
| get_table( |
There was a problem hiding this comment.
I also think by this point we can have get_predictoors_table and get_feeds_table. The "get_table" function used to be simpler buy now it's reached 7 parameters and is almost a pass-through to the table itself
There was a problem hiding this comment.
In my opinion, the get_table method has too many components and it's worth generalizing them rather than repeating them, it also contains other components beside the table, like inputs
| @@ -38,8 +41,8 @@ def predictoor_dash(ppss: PPSS, debug_mode: bool): | |||
| Full error: {e}""" | |||
| ) | |||
| return | |||
|
|
|||
| get_callbacks(app) | |||
| get_callbacks_home(app) | |||
There was a problem hiding this comment.
having a Page-like OOP structure would also help in these cases here. Maybe we can have callbacks autoregistered by each page instead of calling these functions. This comment you can ignore, if you think it's too "science-fiction" :)
There was a problem hiding this comment.
Do you want a structure similar to a modern framework's (Laravel, Django etc) controller?
|
Do you think a Class is needed? |
* Fix #1473: Add navigation and create feeds page (#1475) * separated callbacks to be page specific, views by page * [Fix #1481] PdrDashboard v0.4 Improve multipage DRY (#1482) * Fix #1478: The Feeds Table (#1487) * [Fix #1491] PdrDashboard v0.4 - Split utils.py into concerns (#1493) * [Fix #1490] PdrDashboard v0.4 Fixes predictoor addresses bug if invalid addresses in ppss. (#1492) * [Fix #1485] PdrDashboard v0.4 - Feeds page metrics (#1486) * Fix #1484: Create filters section (#1488) * Fix #1508: Add WS buyer address (#1509) * Fix #1501: Implement Numerize and format helpers (#1505) * [Pdr Dashboard v0.4] Expendable rows (#1507) * Fix #1515: Fix the legend (#1516) * Fix #1520: Add search bar component (#1521) * Fix #1519: Dashduo tests and the "sample_etl" structure (#1522) * tests and architecture fixes
Fixes #1474