Skip to content
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

feat: datasource homepage ui redesign and search functionality for the datasources #38360

Merged
merged 32 commits into from
Jan 9, 2025
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
edecabd
fix: premium ds fixes
AmanAgarwal041 Dec 23, 2024
19d07d8
fix: added calendly link
AmanAgarwal041 Dec 23, 2024
97b2197
feat: display integration in updated ui with search
AmanAgarwal041 Dec 25, 2024
a880be6
feat: resolved conflicts
AmanAgarwal041 Dec 25, 2024
d9defc9
feat: fix errors
AmanAgarwal041 Dec 25, 2024
e5ed096
fix: data test ids
AmanAgarwal041 Dec 26, 2024
aeaeec2
fix: rightsibling flex spacing
AmanAgarwal041 Dec 26, 2024
35f1347
fix: test fixes
AmanAgarwal041 Dec 27, 2024
7c0f440
fix: conflict fixes
AmanAgarwal041 Dec 27, 2024
f652ddd
fix: test fixes
AmanAgarwal041 Dec 27, 2024
fdb2f41
Merge branch 'release' of github.com:appsmithorg/appsmith into feat/s…
AmanAgarwal041 Dec 27, 2024
8e3f1e2
Merge branch 'release' of github.com:appsmithorg/appsmith into feat/s…
AmanAgarwal041 Jan 2, 2025
7801328
fix: onboarding flow ui, cypress test cases
AmanAgarwal041 Jan 2, 2025
bff9896
Merge branch 'release' of github.com:appsmithorg/appsmith into feat/s…
AmanAgarwal041 Jan 2, 2025
307be87
fix: updated the suggestions
AmanAgarwal041 Jan 3, 2025
d1c8430
Merge branch 'release' of github.com:appsmithorg/appsmith into feat/s…
AmanAgarwal041 Jan 3, 2025
fdb8873
fix: moved the divider at correct place
AmanAgarwal041 Jan 3, 2025
df2bd3c
fix: updated the test party bug fixes
AmanAgarwal041 Jan 3, 2025
3ec726c
fix: updated any types
AmanAgarwal041 Jan 3, 2025
17ed083
fix: moved to contact and helpers moved to specific directories
AmanAgarwal041 Jan 6, 2025
5e1877f
Merge branch 'release' of github.com:appsmithorg/appsmith into feat/s…
AmanAgarwal041 Jan 6, 2025
832c305
fix: build error
AmanAgarwal041 Jan 7, 2025
9622d18
Merge branch 'release' of github.com:appsmithorg/appsmith into feat/s…
AmanAgarwal041 Jan 7, 2025
e95fb60
fix: conflict resolve
AmanAgarwal041 Jan 7, 2025
58919a9
fix: deleted the already deleted file
AmanAgarwal041 Jan 7, 2025
4f4ab22
fix: client unit tests
AmanAgarwal041 Jan 7, 2025
e6494bd
fix: ui fixes
AmanAgarwal041 Jan 7, 2025
a06b3fb
Merge branch 'release' of github.com:appsmithorg/appsmith into feat/s…
AmanAgarwal041 Jan 7, 2025
9c34a59
fix: ui fixes
AmanAgarwal041 Jan 8, 2025
028d7f3
Merge branch 'release' of github.com:appsmithorg/appsmith into feat/s…
AmanAgarwal041 Jan 8, 2025
23a91a2
fix: ui fixes banner, spec file
AmanAgarwal041 Jan 8, 2025
aa2489b
Merge branch 'release' of github.com:appsmithorg/appsmith into feat/s…
AmanAgarwal041 Jan 8, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 3 additions & 9 deletions app/client/cypress/e2e/Sanity/Datasources/Styles_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,6 @@ describe(
);
//mock datasource image
cy.datasourceImageStyle("[data-testid=mock-datasource-image]");
//header text
cy.datasourceContentWrapperStyle(".t--datasource-name");
//Name wrapper
cy.get("[data-testid=mock-datasource-name-wrapper]")
.should("have.css", "display", "flex")
Expand All @@ -61,13 +59,9 @@ describe(
"[data-testid=database-datasource-content-wrapper]",
);
//Icon wrapper
cy.datasourceIconWrapperStyle(
"[data-testid=database-datasource-content-wrapper] .dataSourceImage",
);
cy.datasourceIconWrapperStyle("[data-testid=database-datasource-image]");
//Name
cy.datasourceNameStyle(
"[data-testid=database-datasource-content-wrapper] .textBtn",
);
cy.datasourceNameStyle(".t--plugin-name");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Switch to data- attributes for improved stability*
Using a CSS class .t--plugin-name can lead to flaky tests if the class name changes. Consider using a data-* attribute for more robust test selectors.

});

it("3. New API datasource card design", () => {
Expand All @@ -87,7 +81,7 @@ describe(
//Icon wrapper
cy.datasourceIconWrapperStyle(".content-icon");
//Name
cy.datasourceNameStyle(".t--createBlankApiCard .textBtn");
cy.datasourceNameStyle(".t--createBlankApiCard .t--plugin-name");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Use data- attributes to avoid potential flakiness*
Selectors that rely on CSS classes (e.g., .t--createBlankApiCard .t--plugin-name) can easily break if styles change. Opt for data-* attributes to ensure consistent test behavior.

});

after(() => {
Expand Down
2 changes: 1 addition & 1 deletion app/client/cypress/locators/DatasourcesEditor.json
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@
"basicUsername": "input[name='authentication.username']",
"basicPassword": "input[name='authentication.password']",
"mockUserDatabase": "div[id='mock-database'] span:contains('Users')",
"mockUserDatasources": ".t--datasource-name:contains('Users')",
"mockUserDatasources": ".t--plugin-name:contains('Users')",
"mongoUriDropdown": "//p[text()='Use mongo connection string URI']/following-sibling::div",
"mongoUriYes": "//div[text()='Yes']",
"mongoUriInput": "//p[text()='Connection string URI']/following-sibling::div//input",
Expand Down
4 changes: 2 additions & 2 deletions app/client/cypress/support/Pages/DataSources.ts
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ export class DataSources {
_usePreparedStatement =
"input[name='actionConfiguration.pluginSpecifiedTemplates[0].value'][type='checkbox'], input[name='actionConfiguration.formData.preparedStatement.data'][type='checkbox']";
_mockDB = (dbName: string) =>
"//span[text()='" +
"//p[text()='" +
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Avoid using XPath for Cypress tests.
According to the project's Cypress guidelines, it's best practice to avoid XPath selectors and instead rely on data attributes (e.g., data-testid) for more robust and maintainable locators. Consider updating this line to use a data attribute-based selector.

Here's an example diff proposing the change:

- private _mockDB = (dbName: string) =>
-   "//p[text()='" + dbName + "']/ancestor::div[contains(@class, 't--mock-datasource')][1]";
+ private _mockDB = (dbName: string) =>
+   `[data-testid="mock-datasource-${dbName}"]`;

Committable suggestion skipped: line range outside the PR's diff.

dbName +
"']/ancestor::div[contains(@class, 't--mock-datasource')][1]";
private _createBlankGraphQL = ".t--createBlankApiGraphqlCard";
Expand Down Expand Up @@ -203,7 +203,7 @@ export class DataSources {
_queryTimeout = "//input[@name='actionConfiguration.timeoutInMillisecond']";
_getStructureReq = "/api/v1/datasources/*/structure?ignoreCache=true";
_editDatasourceFromActiveTab = (dsName: string) =>
".t--datasource-name:contains('" + dsName + "')";
".t--plugin-name:contains('" + dsName + "')";
_mandatoryMark = "//span[text()='*']";
_deleteDSHostPort = ".t--delete-field";
_dsTabSchema = "[data-testid='t--tab-DATASOURCE_TAB']";
Expand Down
22 changes: 9 additions & 13 deletions app/client/cypress/support/dataSourceCommands.js
Original file line number Diff line number Diff line change
Expand Up @@ -307,32 +307,30 @@ Cypress.Commands.add("datasourceCardContainerStyle", (tag) => {
Cypress.Commands.add("datasourceCardStyle", (tag) => {
cy.get(tag)
.should("have.css", "display", "flex")
.and("have.css", "justify-content", "space-between")
.and("have.css", "align-items", "center")
.and("have.css", "height", "64px")
.and("have.css", "gap", "12px")
.realHover()
.should("have.css", "background-color", backgroundColorGray1)
.and("have.css", "cursor", "pointer");
});

Cypress.Commands.add("datasourceImageStyle", (tag) => {
cy.get(tag)
.should("have.css", "height", "34px")
.should("have.css", "height", "24px")
.and("have.css", "max-width", "100%");
});

Cypress.Commands.add("datasourceContentWrapperStyle", (tag) => {
cy.get(tag)
.should("have.css", "display", "flex")
.and("have.css", "align-items", "center")
.and("have.css", "gap", "13px")
.and("have.css", "padding-left", "13.5px");
.and("have.css", "align-items", "flex-start")
.and("have.css", "gap", "normal");
});

Cypress.Commands.add("datasourceIconWrapperStyle", (tag) => {
cy.get(tag)
.should("have.css", "background-color", backgroundColorGray2)
.and("have.css", "height", "34px")
.and("have.css", "height", "24px")
.and("have.css", "border-radius", "0px")
.and("have.css", "display", "block")
.and("have.css", "align-items", "normal");
Expand All @@ -341,17 +339,15 @@ Cypress.Commands.add("datasourceIconWrapperStyle", (tag) => {
Cypress.Commands.add("datasourceNameStyle", (tag) => {
cy.get(tag)
.should("have.css", "color", backgroundColorBlack)
.and("have.css", "font-size", "16px")
.and("have.css", "font-weight", "400")
.and("have.css", "line-height", "24px")
.and("have.css", "letter-spacing", "-0.24px");
.and("have.css", "font-size", "14px")
.and("have.css", "font-weight", "500")
.and("have.css", "line-height", "20px");
});

Cypress.Commands.add("mockDatasourceDescriptionStyle", (tag) => {
cy.get(tag)
.should("have.css", "color", backgroundColorGray8)
.and("have.css", "font-size", "13px")
.and("have.css", "font-weight", "400")
.and("have.css", "line-height", "17px")
.and("have.css", "letter-spacing", "-0.24px");
.and("have.css", "line-height", "17px");
});
21 changes: 16 additions & 5 deletions app/client/src/ce/constants/messages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -395,6 +395,19 @@ export const CREATE_NEW_DATASOURCE_REST_API = () => "REST API";
export const SAMPLE_DATASOURCES = () => "Sample datasources";
export const EDIT_DS_CONFIG = () => "Edit datasource configuration";
export const NOT_FOUND = () => "Not found";
export const CREATE_NEW_DATASOURCE_AUTHENTICATED_REST_API = () =>
"Authenticated API";
export const CREATE_NEW_DATASOURCE_GRAPHQL_API = () => "GraphQL API";
export const CREATE_NEW_API_SECTION_HEADER = () => "APIs";
export const CREATE_NEW_SAAS_SECTION_HEADER = () => "SaaS integrations";
export const CREATE_NEW_AI_SECTION_HEADER = () => "AI integrations";
export const CONNECT_A_DATASOURCE_HEADING = () => "Connect a datasource";
export const CONNECT_A_DATASOURCE_SUBHEADING = () =>
"Select a sample datasource or connect your own";
export const SEARCH_FOR_DATASOURCES = () => "Search for datasources";
export const EMPTY_SEARCH_DATASOURCES_TITLE = () => "No results found";
export const EMPTY_SEARCH_DATASOURCES_DESCRIPTION = () =>
"Please try again with a different search";

export const ERROR_EVAL_ERROR_GENERIC = () =>
`Unexpected error occurred while evaluating the application`;
Expand Down Expand Up @@ -2322,9 +2335,6 @@ export const START_FROM_SCRATCH_SUBTITLE = () =>
export const START_WITH_DATA_TITLE = () => "Start with data";
export const START_WITH_DATA_SUBTITLE = () =>
"Get started with connecting your data, and easily craft a functional application.";
export const START_WITH_DATA_CONNECT_HEADING = () => "Connect your datasource";
export const START_WITH_DATA_CONNECT_SUBHEADING = () =>
"Select an option to establish a connection. Your data's security is our priority.";
export const START_WITH_TEMPLATE_CONNECT_HEADING = () => "Select a template";
export const START_WITH_TEMPLATE_CONNECT_SUBHEADING = () =>
"Choose an option below to embark on your app-building adventure!";
Expand Down Expand Up @@ -2380,8 +2390,6 @@ export const PARTIAL_IMPORT_EXPORT = {
},
};

export const DATASOURCE_SECURELY_TITLE = () => "Secure & fast connection";

export const CUSTOM_WIDGET_FEATURE = {
addEvent: {
addCTA: () => "Add",
Expand Down Expand Up @@ -2611,3 +2619,6 @@ export const PREMIUM_DATASOURCES = {
"The Appsmith Team is actively working on it. We’ll let you know when this integration is live. ",
NOTIFY_ME: () => "Notify me",
};

export const DATASOURCE_SECURE_TEXT = () =>
`When connecting datasources, your passwords are AES-256 encrypted and we never store any of your data.`;
30 changes: 6 additions & 24 deletions app/client/src/ce/pages/Applications/CreateNewAppsOption.tsx
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
import {
GO_BACK,
SKIP_START_WITH_USE_CASE_TEMPLATES,
START_WITH_DATA_CONNECT_HEADING,
START_WITH_DATA_CONNECT_SUBHEADING,
createMessage,
} from "ee/constants/messages";
import urlBuilder from "ee/entities/URLRedirect/URLAssembly";
Expand All @@ -12,7 +10,7 @@ import {
resetCurrentPluginIdForCreateNewApp,
} from "actions/onboardingActions";
import { fetchPlugins } from "actions/pluginActions";
import { Flex, Link, Text } from "@appsmith/ads";
import { Flex, Link } from "@appsmith/ads";
import CreateNewDatasourceTab from "pages/Editor/IntegrationEditor/CreateNewDatasourceTab";
import { getApplicationsOfWorkspace } from "ee/selectors/selectedWorkspaceSelectors";
import { default as React, useEffect } from "react";
Expand All @@ -36,7 +34,6 @@ import { isAirgapped } from "ee/utils/airgapHelpers";
const SectionWrapper = styled.div<{ isBannerVisible: boolean }>`
display: flex;
flex-direction: column;
padding: 0 var(--ads-v2-spaces-7) var(--ads-v2-spaces-7);
${(props) => `
margin-top: ${
props.theme.homePage.header + (props.isBannerVisible ? 40 : 0)
Expand All @@ -56,8 +53,9 @@ const BackWrapper = styled.div<{ hidden?: boolean; isBannerVisible: boolean }>`
top: ${props.theme.homePage.header + (props.isBannerVisible ? 40 : 0)}px;
`}
background: inherit;
padding: var(--ads-v2-spaces-3);
padding: var(--ads-v2-spaces-3) var(--ads-v2-spaces-8);
z-index: 1;
border-bottom: 1px solid var(--ads-v2-color-gray-300);
margin-left: -4px;
${(props) => `${props.hidden && "visibility: hidden; opacity: 0;"}`}
`;
Expand All @@ -66,22 +64,10 @@ const LinkWrapper = styled(Link)<{ hidden?: boolean }>`
${(props) => `${props.hidden && "visibility: hidden; opacity: 0;"}`}
`;

const WithDataWrapper = styled.div`
const WithDataWrapper = styled(Flex)`
background: var(--ads-v2-color-bg);
padding: var(--ads-v2-spaces-13);
border: 1px solid var(--ads-v2-color-gray-300);
border-radius: 5px;
`;

const Header = ({ subtitle, title }: { subtitle: string; title: string }) => {
return (
<Flex flexDirection="column" mb="spaces-14" mt="spaces-7">
<Text kind="heading-xl">{title}</Text>
<Text>{subtitle}</Text>
</Flex>
);
};

const CreateNewAppsOption = ({
currentApplicationIdForCreateNewApp,
}: {
Expand Down Expand Up @@ -227,12 +213,8 @@ const CreateNewAppsOption = ({
</LinkWrapper>
)}
</BackWrapper>
<Flex flexDirection="column" pl="spaces-3" pr="spaces-3">
<Header
subtitle={createMessage(START_WITH_DATA_CONNECT_SUBHEADING)}
title={createMessage(START_WITH_DATA_CONNECT_HEADING)}
/>
<WithDataWrapper>
<Flex flex={"1"} flexDirection="column">
<WithDataWrapper flex={"1"} flexDirection="column">
{createNewAppPluginId && !!selectedDatasource ? (
selectedPlugin?.type === PluginType.SAAS ? (
<DatasourceForm
Expand Down
2 changes: 1 addition & 1 deletion app/client/src/constants/PremiumDatasourcesConstants.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { getAssetUrl } from "ee/utils/airgapHelpers";
import { ASSETS_CDN_URL } from "./ThirdPartyConstants";

interface PremiumIntegration {
export interface PremiumIntegration {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AmanAgarwal041 Instead of creating this const file inside the common constants folder, can we create a constants folder inside IntegrationEditor and keep it there. I believe these constants are not being used outside of IntegrationEditor, that way we will have a better organisation of files, WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets keep it here only, as if we keep the constants inside the IntegrationEditor then we should ideally keep the utils PremiumDatasourcesHelpers also inside IntegrationEditor. But that would not be a good option here. Keeping these things segregated is good.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should even keep utils inside IntegrationEditor, that way we would have all things needed for IntegrationEditor inside IntegrationEditor folder, following modularisation framework.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have anything adopted yet ? If not then we should leave it for now, otherwise we would be in a state where we follow two architecture mechanism at once, which becomes a little confusing.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

name: string;
icon: string;
}
Expand Down
Loading
Loading