From 3b3b3b44762b20316128d01e346b4a593f4f3fa2 Mon Sep 17 00:00:00 2001 From: sneha122 Date: Tue, 13 Feb 2024 16:44:59 +0530 Subject: [PATCH] fix: disabled feature walkthroughs behind a flag (#31007) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description This PR removes the feature walkthroughs that were added as part of one of the activation experiments in Q3. Although these walkthroughs were useful in guiding users, they were appearing in untimely and incorrect fashion. #### PR fixes following issue(s) Fixes #30339 #### Media > A video or a GIF is preferred. when using Loom, don’t embed because it looks like it’s a GIF. instead, just link to the video > > #### Type of change > Please delete options that are not relevant. - Bug fix (non-breaking change which fixes an issue) > > > ## Testing > #### How Has This Been Tested? > Please describe the tests that you ran to verify your changes. Also list any relevant details for your test configuration. > Delete anything that is not relevant - [x] Manual - [ ] JUnit - [ ] Jest - [ ] Cypress > > #### Test Plan > Add Testsmith test cases links that relate to this PR > > #### Issues raised during DP testing > Link issues raised during DP testing for better visiblity and tracking (copy link from comments dropped on this PR) > > > ## Checklist: #### Dev activity - [ ] My code follows the style guidelines of this project - [ ] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [ ] My changes generate no new warnings - [ ] I have added tests that prove my fix is effective or that my feature works - [ ] New and existing unit tests pass locally with my changes - [ ] PR is being merged under a feature flag #### QA activity: - [ ] [Speedbreak features](https://github.com/appsmithorg/TestSmith/wiki/Guidelines-for-test-plans#speedbreakers-) have been covered - [ ] Test plan covers all impacted features and [areas of interest](https://github.com/appsmithorg/TestSmith/wiki/Guidelines-for-test-plans#areas-of-interest-) - [ ] Test plan has been peer reviewed by project stakeholders and other QA members - [ ] Manually tested functionality on DP - [ ] We had an implementation alignment call with stakeholders post QA Round 2 - [ ] Cypress test cases have been added and approved by SDET/manual QA - [ ] Added `Test Plan Approved` label after Cypress tests were reviewed - [ ] Added `Test Plan Approved` label after JUnit tests were reviewed ## Summary by CodeRabbit - **New Features** - Introduced a feature flag to toggle the feature walkthrough on or off. - Added the ability to force display the feature walkthrough regardless of the feature flag setting. --------- Co-authored-by: “sneha122” <“sneha@appsmith.com”> --- app/client/src/ce/entities/FeatureFlag.ts | 3 ++ .../components/featureWalkthrough/index.tsx | 31 +++++++++++++------ .../featureWalkthrough/walkthroughContext.tsx | 3 ++ 3 files changed, 28 insertions(+), 9 deletions(-) diff --git a/app/client/src/ce/entities/FeatureFlag.ts b/app/client/src/ce/entities/FeatureFlag.ts index b4bdf28ee7..f4a24174e8 100644 --- a/app/client/src/ce/entities/FeatureFlag.ts +++ b/app/client/src/ce/entities/FeatureFlag.ts @@ -51,6 +51,8 @@ export const FEATURE_FLAG = { rollout_editor_pane_segments_enabled: "rollout_editor_pane_segments_enabled", release_show_create_app_from_templates_enabled: "release_show_create_app_from_templates_enabled", + rollout_remove_feature_walkthrough_enabled: + "rollout_remove_feature_walkthrough_enabled", } as const; export type FeatureFlag = keyof typeof FEATURE_FLAG; @@ -92,6 +94,7 @@ export const DEFAULT_FEATURE_FLAG_VALUE: FeatureFlags = { release_actions_redesign_enabled: false, rollout_editor_pane_segments_enabled: false, release_show_create_app_from_templates_enabled: false, + rollout_remove_feature_walkthrough_enabled: false, }; export const AB_TESTING_EVENT_KEYS = { diff --git a/app/client/src/components/featureWalkthrough/index.tsx b/app/client/src/components/featureWalkthrough/index.tsx index d9353125c6..9dd792b4a6 100644 --- a/app/client/src/components/featureWalkthrough/index.tsx +++ b/app/client/src/components/featureWalkthrough/index.tsx @@ -8,6 +8,10 @@ import { useLocation } from "react-router-dom"; import AnalyticsUtil from "utils/AnalyticsUtil"; import { isElementVisible } from "./utils"; import { hideIndicator } from "components/utils/Indicator"; +import { FEATURE_FLAG } from "@appsmith/entities/FeatureFlag"; +import { selectFeatureFlagCheck } from "@appsmith/selectors/featureFlagsSelectors"; +import { useSelector } from "react-redux"; +import type { AppState } from "@appsmith/reducers"; const WalkthroughRenderer = lazy(async () => { return retryPromise( @@ -26,18 +30,27 @@ export default function Walkthrough({ children }: any) { const [feature, setFeature] = useState([]); const location = useLocation(); + const isWalkthroughDisabled = useSelector((state: AppState) => + selectFeatureFlagCheck( + state, + FEATURE_FLAG.rollout_remove_feature_walkthrough_enabled, + ), + ); + const pushFeature = (value: FeatureParams, prioritize = false) => { - const alreadyExists = feature.some((f) => f.targetId === value.targetId); - if (!alreadyExists) { - const _value = Array.isArray(value) ? [...value] : [value]; - if (prioritize) { - // Get ahead of the queue - setFeature((e) => [..._value, ...e]); - } else { - setFeature((e) => [...e, ..._value]); + if (!isWalkthroughDisabled || !!value?.forceExecution) { + const alreadyExists = feature.some((f) => f.targetId === value.targetId); + if (!alreadyExists) { + const _value = Array.isArray(value) ? [...value] : [value]; + if (prioritize) { + // Get ahead of the queue + setFeature((e) => [..._value, ...e]); + } else { + setFeature((e) => [...e, ..._value]); + } } + updateActiveWalkthrough(); } - updateActiveWalkthrough(); }; const popFeature = (triggeredFrom?: string) => { diff --git a/app/client/src/components/featureWalkthrough/walkthroughContext.tsx b/app/client/src/components/featureWalkthrough/walkthroughContext.tsx index 9b7048ce5b..322b9137f0 100644 --- a/app/client/src/components/featureWalkthrough/walkthroughContext.tsx +++ b/app/client/src/components/featureWalkthrough/walkthroughContext.tsx @@ -67,6 +67,9 @@ export interface FeatureParams { dismissOnOverlayClick?: boolean; // execute function just before showing walkthrough highlight runBeforeWalkthrough?: () => void; + // If we want to force the display of walkthrough, set this prop to true + // If set to true, will override the feature flag as well + forceExecution?: boolean; } interface WalkthroughContextType {