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.

Sign up for GitHub

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

Jump to bottom

Hide "try demo" on login page (fixes #2106) #3889

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Hide "try demo" on login page (fixes #2106) #3889

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
47ab2c5
Adds environment variable to hide "try demo" button on the login page
sammichaels Nov 23, 2024
ab416e4
Fix indentation
sammichaels Nov 23, 2024
b6d6056
Clarification that this is for the login page only
sammichaels Nov 24, 2024
0e3fdd2
Merge branch 'actualbudget:master' into no-demo
sammichaels Nov 24, 2024
7a1ab54
Release notes
sammichaels Nov 24, 2024
bf13c8d
lint
sammichaels Nov 24, 2024
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
31 changes: 17 additions & 14 deletions packages/desktop-client/src/components/manager/subscribe/Login.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { useParams, useSearchParams } from 'react-router-dom';
import { createBudget } from 'loot-core/src/client/actions/budgets';
import { loggedIn } from 'loot-core/src/client/actions/user';
import { send } from 'loot-core/src/platform/client/fetch';
import { shouldDisplayLoginDemo } from 'loot-core/src/shared/environment';

import { AnimatedLoading } from '../../../icons/AnimatedLoading';
import { theme } from '../../../style';
Expand Down Expand Up @@ -169,21 +170,23 @@ export function Login() {
)}
</View>
)}
<View
style={{
flexDirection: 'row',
justifyContent: 'center',
marginTop: 15,
}}
>
<Button
variant="bare"
style={{ fontSize: 15, color: theme.pageTextLink, marginLeft: 10 }}
onPress={onDemo}
{shouldDisplayLoginDemo() && (
<View
style={{
flexDirection: 'row',
justifyContent: 'center',
marginTop: 15,
}}
>
<Trans>Try Demo &rarr;</Trans>
</Button>
</View>
<Button
variant="bare"
style={{ fontSize: 15, color: theme.pageTextLink, marginLeft: 10 }}
onPress={onDemo}
>
<Trans>Try Demo &rarr;</Trans>
</Button>
</View>
)}
</View>
);
}
4 changes: 4 additions & 0 deletions packages/loot-core/src/shared/environment.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,3 +18,7 @@ export function isElectron() {
}
return false;
}

export function shouldDisplayLoginDemo() {
return process.env.REACT_APP_HIDE_LOGIN_DEMO === undefined;
}
Comment on lines +22 to +24
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

Environment variable needs documentation

The implementation is correctly used in the Login component to conditionally render a demo section, but the REACT_APP_HIDE_LOGIN_DEMO environment variable is not documented anywhere in the codebase. This could cause confusion for other developers.

  • Add documentation for REACT_APP_HIDE_LOGIN_DEMO environment variable in packages/desktop-client/README.md, explaining its purpose and usage
  • Consider adding a .env.example file to showcase the available environment variables
🔗 Analysis chain

Verify implementation completeness.

Let's verify the implementation and documentation:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the implementation completeness

# Check if the function is used in the Login component
echo "Checking function usage in Login component..."
rg -l "shouldDisplayLoginDemo" "packages/desktop-client/src/components/manager/subscribe/Login"

# Check for any other usages of the function
echo "Checking for other function usages..."
rg -l "shouldDisplayLoginDemo"

# Check if environment variable is documented
echo "Checking for environment variable documentation..."
rg -l "REACT_APP_HIDE_LOGIN_DEMO" -g "*.md"
rg -l "REACT_APP_HIDE_LOGIN_DEMO" -g "*.env*"

Length of output: 875

Script:

#!/bin/bash
# Let's examine the actual usage of the function in the Login component
echo "Checking function implementation in Login.tsx..."
rg "shouldDisplayLoginDemo" packages/desktop-client/src/components/manager/subscribe/Login.tsx -A 3 -B 3

# Let's also check for any environment variable setup files
echo "Checking for environment setup files..."
fd -g ".env*"

# Check for any documentation files that might contain environment variables
echo "Checking documentation files..."
fd "README.md"

Length of output: 1254

Script:

#!/bin/bash
# Let's check the content of relevant README files for environment variable documentation
echo "Checking README files for environment documentation..."
rg "REACT_APP_HIDE_LOGIN_DEMO|environment|env" packages/desktop-client/README.md -A 3 -B 3

# Let's check with hidden files included for .env files
echo "Checking for .env files with hidden flag..."
fd -H -g ".env*"

# Let's also check the Login component to see the complete context of the demo usage
echo "Checking Login component demo section..."
rg "shouldDisplayLoginDemo" packages/desktop-client/src/components/manager/subscribe/Login.tsx -A 10 -B 3

Length of output: 2007

6 changes: 6 additions & 0 deletions upcoming-release-notes/3889.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
category: Features
authors: [sammichaels]
---

Adds environment variable to remove "try demo" link on login page.