feat: Add "Start with template" option and refactor template components #30946
Conversation
…o feat/30859/revert-templates-exp-in-homepage
|
/build-deploy-preview |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/7810603206. |
|
/ok-to-test tags="@tag.templates" |
|
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/7810631145. |
|
Deploy-Preview-URL: https://ce-30946.dp.appsmith.com |
|
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/7810631145.
To know the list of identified flaky tests - Refer here |
|
/build-deploy-preview skip-tests=true recreate=false |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/7811038111. |
|
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/7810631145.
To know the list of identified flaky tests - Refer here |
|
Deploy-Preview-URL: https://ce-30946.dp.appsmith.com |
github-actions
bot
added
Templates
Templates Product
|
/build-deploy-preview |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
| interface HeaderProps { | ||
| subtitle: string; | ||
| title: string; | ||
| isModalLayout?: boolean; |
There was a problem hiding this comment.
The isModalLayout prop in StartWithTemplatesHeader should have a default value to ensure backward compatibility and prevent potential issues when the prop is not explicitly provided. Consider setting a default value that reflects the most common usage scenario.
- isModalLayout?: boolean;
+ isModalLayout: boolean = false;Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| isModalLayout?: boolean; | |
| isModalLayout: boolean = false; |
| <Flex | ||
| flexDirection="column" | ||
| mb={isModalLayout ? "spaces-5" : "spaces-14"} | ||
| mt={isModalLayout ? "" : "spaces-7"} |
There was a problem hiding this comment.
The conditional margin-bottom (mb) based on isModalLayout is a good approach for responsive design. However, ensure that the empty string assignment for mt when isModalLayout is true does not lead to unintended layout issues. It might be safer to explicitly set a value or use conditional rendering to include or exclude the margin-top property.
- mt={isModalLayout ? "" : "spaces-7"}
+ mt={isModalLayout ? "0" : "spaces-7"}Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| mt={isModalLayout ? "" : "spaces-7"} | |
| mt={isModalLayout ? "0" : "spaces-7"} |
|
/ok-to-test tags="@tag.Templates" |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/7867588162. |
|
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/7867590515. |
|
Deploy-Preview-URL: https://ce-30946.dp.appsmith.com |
|
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/7867590515.
To know the list of identified flaky tests - Refer here |
|
/build-deploy-preview |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/7867825703. |
|
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/7867590515.
To know the list of identified flaky tests - Refer here |
|
Deploy-Preview-URL: https://ce-30946.dp.appsmith.com |
|
/build-deploy-preview |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/7868153710. |
|
Deploy-Preview-URL: https://ce-30946.dp.appsmith.com |
|
/build-deploy-preview |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/7868395498. |
| import TemplatesLayoutWithFilters from "pages/Templates/TemplatesLayoutWithFilters"; | ||
| import React from "react"; | ||
| import { useDispatch, useSelector } from "react-redux"; | ||
| import { | ||
| getForkableWorkspaces, | ||
| isImportingTemplateToAppSelector, | ||
| } from "selectors/templatesSelectors"; | ||
| import { useSelector } from "react-redux"; | ||
| import { getForkableWorkspaces } from "selectors/templatesSelectors"; |
There was a problem hiding this comment.
Consider grouping imports from the same library or module together to improve readability and maintainability.
import React from "react";
import { useSelector } from "react-redux";
import { getForkableWorkspaces } from "selectors/templatesSelectors";
import styled from "styled-components";Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| import TemplatesLayoutWithFilters from "pages/Templates/TemplatesLayoutWithFilters"; | |
| import React from "react"; | |
| import { useDispatch, useSelector } from "react-redux"; | |
| import { | |
| getForkableWorkspaces, | |
| isImportingTemplateToAppSelector, | |
| } from "selectors/templatesSelectors"; | |
| import { useSelector } from "react-redux"; | |
| import { getForkableWorkspaces } from "selectors/templatesSelectors"; | |
| import TemplatesLayoutWithFilters from "pages/Templates/TemplatesLayoutWithFilters"; | |
| import React from "react"; | |
| import { useSelector } from "react-redux"; | |
| import { getForkableWorkspaces } from "selectors/templatesSelectors"; | |
| import styled from "styled-components"; |
| onForkTemplateClick: (template: Template) => void; | ||
| isInsideModal?: boolean; |
There was a problem hiding this comment.
The isInsideModal prop is declared but not used within StartWithTemplatesWrapper. If it's intended for future use, ensure it's documented; otherwise, consider removing it to avoid confusion.
| <TemplatesLayoutWithFilters | ||
| analyticsEventNameForTemplateCardClick="CLICK_ON_TEMPLATE_CARD_WHEN_ONBOARDING" |
There was a problem hiding this comment.
The analyticsEventNameForTemplateCardClick prop in TemplatesLayoutWithFilters is hardcoded. Consider making this value configurable through props or a constants file to enhance flexibility and maintainability.
| interface TemplatesLayoutWithFilterProps { | ||
| initialFilters?: Record<string, string[]>; | ||
| isForkingEnabled?: boolean; | ||
| isModalLayout?: boolean; | ||
| setSelectedTemplate: (id: string) => void; | ||
| onForkTemplateClick: (template: TemplateInterface) => void; | ||
| analyticsEventNameForTemplateCardClick: EventName; |
There was a problem hiding this comment.
The analyticsEventNameForTemplateCardClick prop is a good addition for custom analytics events. However, ensure that this prop is documented, especially regarding its expected values and usage, to aid in maintainability and clarity for future developers.
|
Deploy-Preview-URL: https://ce-30946.dp.appsmith.com |
…ease' of https://github.com/appsmithorg/appsmith into feat/30859/revert-templates-exp-in-homepage
Description
This pull request adds the "Start with template" option to the workspace action menu and refactors several template components to improve code organization and maintainability.
PR fixes following issue(s)
Fixes #30860
Media
Type of change
Testing
How Has This Been Tested?
Test Plan
Issues raised during DP testing
Checklist:
Dev activity
QA activity:
Test Plan Approvedlabel after Cypress tests were reviewedTest Plan Approvedlabel after JUnit tests were reviewedSummary by CodeRabbit