preview tab for copilot #726
Conversation
📝 WalkthroughWalkthroughAdds a new RecipeTabs.preview tab and a preview_route, registers PREVIEW_ROUTE_WORKFLOWS, and integrates preview into routing and UI. daras_ai_v2/base.py: introduces NAV_TABS_CSS and INPUT_OUTPUT_COLS_CSS, updates tab rendering/styling (responsive scrolling, active-lg indicator), treats preview like run for two-column input/output layout, conditionally hides input or output on large screens, adjusts header/social button logic, and redirects submits to preview for workflows in PREVIEW_ROUTE_WORKFLOWS. daras_ai_v2/meta_content.py: refactors robots_tag_for_page to a match on page.tab and adds explicit handling for RecipeTabs.preview. recipes/VideoBots.py: inserts the Preview tab, shows demo header buttons on Run and Preview, and updates running-output scroll target. routers/root.py: adds preview_route, PREVIEW_ROUTE_WORKFLOWS, and a RecipeTabs.preview enum member; adjusts some tab titles. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15–25 minutes Possibly related PRs
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
anish-work
force-pushed
the
preview_tab_mobile
branch
from
ad0ea84 to
72ff56c
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
recipes/VideoBots.py (1)
1482-1483: Refactor the comparison for better readability.The logic is correct but can be improved as suggested by static analysis.
Apply this diff to use the
inoperator:- if self.tab == RecipeTabs.run or self.tab == RecipeTabs.preview: + if self.tab in (RecipeTabs.run, RecipeTabs.preview):daras_ai_v2/base.py (2)
480-480: Refactor to use 'in' operator for better readability.Consider using the
inoperator for cleaner code:- if self.tab != RecipeTabs.run and self.tab != RecipeTabs.preview: + if self.tab not in (RecipeTabs.run, RecipeTabs.preview):
1839-1844: Good implementation with minor style improvement needed.The conditional styling logic correctly implements the preview functionality. Consider using a dict literal for better performance:
- style=dict(position="sticky", top="0.5rem"), + style={"position": "sticky", "top": "0.5rem"},
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
daras_ai_v2/base.py(6 hunks)daras_ai_v2/meta_content.py(1 hunks)recipes/VideoBots.py(1 hunks)routers/root.py(3 hunks)
🧰 Additional context used
🪛 Pylint (3.3.7)
recipes/VideoBots.py
[refactor] 1482-1482: Consider merging these comparisons with 'in' by using 'self.tab in (RecipeTabs.run, RecipeTabs.preview)'. Use a set instead if elements are hashable.
(R1714)
daras_ai_v2/base.py
[refactor] 480-480: Consider merging these comparisons with 'in' by using 'self.tab not in (RecipeTabs.run, RecipeTabs.preview)'. Use a set instead if elements are hashable.
(R1714)
[refactor] 1840-1840: Consider using '{"position": 'sticky', "top": '0.5rem'}' instead of a call to 'dict'.
(R1735)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test (3.10.12, 1.8.3)
🔇 Additional comments (11)
daras_ai_v2/meta_content.py (1)
223-228: LGTM! Consistent implementation of preview tab meta tag logic.The new preview tab cases correctly mirror the existing run tab logic, maintaining consistency in SEO handling between the two tabs.
recipes/VideoBots.py (1)
1487-1487: LGTM! Preview tab correctly inserted into tabs list.The preview tab is properly positioned as the second tab after run, which makes logical sense for the user experience.
routers/root.py (3)
595-605: LGTM! Preview route correctly implemented.The new preview route follows the established pattern for tab routes and properly delegates to
render_recipe_pagewith the preview tab.
842-842: LGTM! Title simplifications improve consistency.Removing the responsive span wrappers simplifies the tab titles and maintains consistency with the new preview tab implementation.
Also applies to: 862-862
836-840: No action required: ‘mobile-only-recipe-tab’ CSS rules are presentThe
mobile-only-recipe-tabselector is already defined in daras_ai_v2/base.py around lines 383 and 399, toggling its display on mobile vs. desktop:
- Lines 383–385:
& a:has(span.mobile-only-recipe-tab) { display: block !important; }- Lines 399–401:
@media (min-width: 768px) { & a:has(span.mobile-only-recipe-tab) { display: none !important; } }daras_ai_v2/base.py (6)
382-412: LGTM! Well-implemented responsive CSS for preview tab functionality.The CSS additions properly handle the responsive behavior for the new preview tab, including mobile-only visibility controls, horizontal scrollable tabs, and special active state styling for large screens.
420-430: LGTM! Correct implementation of conditional tab styling.The tab rendering logic properly adds the "active-lg" class to the run tab when the current tab is preview, enabling the CSS styling for the preview mode visual feedback.
1204-1204: LGTM! Correct pattern matching for preview and run tabs.The update properly handles both
runandpreviewtabs with the same rendering logic, which is appropriate for the preview functionality.
1766-1787: LGTM! Proper implementation of input form hiding in preview mode.The conditional visibility logic correctly hides the input form on large screens when the current tab is preview, maintaining the appropriate responsive behavior for the preview functionality.
2536-2536: LGTM! Well-defined constant for preview route workflows.The constant is properly defined and allows for easy extension to other workflows that should support preview route behavior.
2537-2548: LGTM! Simplified and focused CSS styling.The OUTPUT_TABS_CSS has been appropriately simplified and updated to support the new preview tab functionality with proper responsive styling.
anish-work
force-pushed
the
preview_tab_mobile
branch
from
ca313bd to
c653f47
Compare
daras_ai_v2/base.py
Outdated
| if self.workflow in PREVIEW_ROUTE_WORKFLOWS: | ||
| raise gui.RedirectException(pr.get_app_url()) | ||
| else: | ||
| raise gui.RedirectException(pr.get_app_url()) |
There was a problem hiding this comment.
was unintended, removed now..
anish-work
force-pushed
the
preview_tab_mobile
branch
from
806e9f5 to
5e89f66
Compare
|
can you merge pls |
@devxpy done |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
routers/root.py (1)
891-892: Consider making PREVIEW_ROUTE_WORKFLOWS immutable.Since this is a module-level constant that shouldn't be modified at runtime, consider using a tuple instead of a list for immutability.
-PREVIEW_ROUTE_WORKFLOWS = [Workflow.VIDEO_BOTS] +PREVIEW_ROUTE_WORKFLOWS = (Workflow.VIDEO_BOTS,)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
daras_ai_v2/base.py(10 hunks)daras_ai_v2/meta_content.py(1 hunks)recipes/VideoBots.py(2 hunks)routers/root.py(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- recipes/VideoBots.py
- daras_ai_v2/meta_content.py
🧰 Additional context used
🧬 Code Graph Analysis (2)
daras_ai_v2/base.py (2)
recipes/VideoBotsStats.py (1)
current_app_url(75-81)routers/root.py (1)
RecipeTabs(894-938)
routers/root.py (2)
bots/models/workflow.py (1)
Workflow(189-257)routers/account.py (1)
TabData(246-248)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: test (3.10.12, 1.8.3)
- GitHub Check: Analyze (python)
🔇 Additional comments (6)
routers/root.py (1)
586-596: LGTM! Preview route follows established patterns.The new preview route is well-implemented and consistent with other route handlers in the file.
daras_ai_v2/base.py (5)
451-456: Consider clarifying the active tab UI logic.The current implementation shows the Run tab as active on large screens when the Preview tab is selected. This could be confusing for users as it creates an inconsistency between what's actually active and what appears active.
Consider either:
- Making the Preview tab visible on all screen sizes when it's active
- Adding a comment explaining why this UX pattern was chosen
- Using a different visual indicator for this special case
1962-1967: LGTM! Clean implementation of preview route redirect.The conditional redirect logic properly routes workflows to the preview tab when they're in PREVIEW_ROUTE_WORKFLOWS.
410-443: Well-structured responsive CSS for tab navigation.The CSS properly handles:
- Horizontal scrolling for tabs on mobile
- Hiding mobile-only tabs on larger screens
- Active state styling for the preview/run tab relationship
1812-1813: LGTM! Proper conditional visibility for preview mode.The conditional CSS classes correctly hide/show input and output columns based on the preview tab state and workflow membership in PREVIEW_ROUTE_WORKFLOWS.
Also applies to: 1884-1889
2617-2639: Cleaner and more maintainable CSS structure.The simplified OUTPUT_TABS_CSS with proper responsive breakpoints and clearer padding/margin rules improves maintainability.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
daras_ai_v2/base.py (1)
395-407: Improve the tab rendering logic for better maintainability.The logic for determining when to show the
active-lgindicator is complex and could be simplified. The current implementation creates a conditionalforce_active_lgelement based on whether the current tab is preview and the loop tab is run.Consider refactoring this logic for better readability:
- if tab == RecipeTabs.run and self.tab == RecipeTabs.preview: - force_active_lg = gui.tag("span", className="active-lg") - else: - force_active_lg = gui.dummy() - with force_active_lg, gui.nav_item(url, active=tab == self.tab): + should_force_active = tab == RecipeTabs.run and self.tab == RecipeTabs.preview + with gui.nav_item(url, active=tab == self.tab): + if should_force_active: + with gui.tag("span", className="active-lg"): + gui.html(tab.title) + else: + gui.html(tab.title)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
daras_ai_v2/base.py(10 hunks)routers/root.py(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- routers/root.py
🧰 Additional context used
🧬 Code Graph Analysis (1)
daras_ai_v2/base.py (2)
recipes/VideoBotsStats.py (1)
current_app_url(75-81)routers/root.py (1)
RecipeTabs(894-938)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Analyze (python)
- GitHub Check: test (3.10.12, 1.8.3)
🔇 Additional comments (9)
daras_ai_v2/base.py (9)
92-93: LGTM!The import of
PREVIEW_ROUTE_WORKFLOWSis correctly placed and follows proper import organization.
425-425: LGTM!The condition correctly excludes both
runandpreviewtabs from the zero-margin title block, maintaining consistent UI behavior.
602-602: LGTM!The extension of social button rendering to include the preview tab maintains UI consistency between run and preview modes.
1166-1166: LGTM!Using pattern matching with the union operator correctly treats both
runandpreviewtabs the same way for the two-column layout.
1753-1776: LGTM!The conditional hiding of the input column on large screens when the tab is preview provides a clean responsive design. The logic correctly preserves all other functionality while adding the responsive behavior.
1828-1834: LGTM!The responsive hiding logic for the output column correctly shows/hides based on the tab and workflow type, maintaining a good user experience across different screen sizes.
1907-1911: LGTM!The redirect logic correctly routes to the preview tab for workflows in
PREVIEW_ROUTE_WORKFLOWS, providing a seamless user experience for preview-enabled workflows.
2561-2633: LGTM!The CSS definitions are well-structured and provide good responsive behavior:
INPUT_OUTPUT_COLS_CSSproperly handles the two-column layout with appropriate padding and background stylingNAV_TABS_CSSincludes comprehensive responsive navigation styling with horizontal scrolling on mobile and proper active state indicatorsThe CSS follows best practices with proper media queries and maintains visual consistency across devices.
1194-1199: Consistent CSS class naming verifiedAll
OUTPUT_TABS_CSSreferences have been removed and replaced withINPUT_OUTPUT_COLS_CSS, and no occurrences of the old constant remain in the codebase. No further changes are needed.
Q/A checklist
How to check import time?
You can visualize this using tuna:
To measure import time for a specific library:
To reduce import times, import libraries that take a long time inside the functions that use them instead of at the top of the file:
Legal Boilerplate
Look, I get it. The entity doing business as “Gooey.AI” and/or “Dara.network” was incorporated in the State of Delaware in 2020 as Dara Network Inc. and is gonna need some rights from me in order to utilize my contributions in this PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Dara Network Inc can use, modify, copy, and redistribute my contributions, under its choice of terms.