Repository: joinmarket-webui/joinmarket-webui
Pull request #239 (2022-05-06 09:03:19)

ui: decrease brightness of placeholder in light theme
It feels that the placeholders in light mode are little bit too bright. On some screens it is hard to tell the difference from the background. Do you think it would be an improvement if they were a little darker?

## 📸 Before
![image](https://user-images.githubusercontent.com/3358649/167100624-e65889b5-a399-4b7e-9552-07b2191616ca.png)

## 📸 After
![image](https://user-images.githubusercontent.com/3358649/167100640-2fb0225b-13e9-4422-a601-8e4e3de98298.png)


Just for reference, this would be the original Bootstrap style (applying `currentColor`):
![image](https://user-images.githubusercontent.com/3358649/167100659-d1432a10-0997-4ad7-8b27-02983d3fb488.png)


editwentyone: its ok, go a little darker, but not that dark as in the last screenshot. a spinner per block would also help to visualize that something is loading
theborakompanioni: > its ok, go a little darker, but not that dark as in the last screenshot. a spinner per block would also help to visualize that something is loading

Would you approve the PR if you are happy with the changes? :orange_heart: 

Patch:
diff --git a/src/components/Balance.jsx b/src/components/Balance.jsx
index cc7b002..fba1374 100644
--- a/src/components/Balance.jsx
+++ b/src/components/Balance.jsx
@@ -68,13 +68,25 @@ const BalanceComponent = ({ symbol, value, symbolIsPrefix }) => {
  * @param {showBalance}: A flag indicating whether to render or hide the balance.
  * Hidden balances are masked with `*****`.
  * @param {loading}: A loading flag that renders a placeholder while true.
+ * @param {enableVisibilityToggle}: A flag that controls whether the balance can mask/unmask when clicked
  */
-export default function Balance({ valueString, convertToUnit, showBalance = false, loading = false }) {
+export default function Balance({
+  valueString,
+  convertToUnit,
+  showBalance = false,
+  loading = false,
+  enableVisibilityToggle = !showBalance,
+}) {
   const [displayMode, setDisplayMode] = useState(DISPLAY_MODE_HIDDEN)
+  const [isBalanceVisible, setIsBalanceVisible] = useState(showBalance)
 
   useEffect(() => {
-    setDisplayMode(getDisplayMode(convertToUnit, showBalance))
-  }, [convertToUnit, showBalance])
+    setIsBalanceVisible(showBalance)
+  }, [showBalance])
+
+  useEffect(() => {
+    setDisplayMode(getDisplayMode(convertToUnit, isBalanceVisible))
+  }, [convertToUnit, isBalanceVisible])
 
   if (loading) {
     return (
@@ -87,43 +99,62 @@ export default function Balance({ valueString, convertToUnit, showBalance = fals
     )
   }
 
-  if (displayMode === DISPLAY_MODE_HIDDEN) {
-    return (
-      <BalanceComponent
-        symbol={
-          <span className="d-inline-flex align-items-center text-muted">
-            <Sprite symbol="hide" width="1.2em" height="1.2em" className="ps-1" />
-          </span>
-        }
-        value={'*****'}
-        symbolIsPrefix={false}
-      />
-    )
-  }
+  const toggleVisibility = (e) => {
+    e.preventDefault()
+    e.stopPropagation()
 
-  if (typeof valueString !== 'string') {
-    console.warn('<Balance /> component expects string input')
-    return <BalanceComponent symbol="" value={valueString} symbolIsPrefix={false} />
+    setIsBalanceVisible((current) => !current)
   }
 
-  // Treat integers as sats.
-  const valueIsSats = valueString === Number.parseInt(valueString).toString()
-  // Treat decimal numbers as btc.
-  const valueIsBtc = !valueIsSats && !Number.isNaN(Number.parseFloat(valueString)) && valueString.indexOf('.') > -1
-
-  const btcSymbol = <span style={{ paddingRight: '0.1em' }}>{'\u20BF'}</span>
-  const satSymbol = <Sprite symbol="sats" width="1.2em" height="1.2em" />
-
-  if (valueIsBtc && displayMode === DISPLAY_MODE_BTC)
-    return <BalanceComponent symbol={btcSymbol} value={formatBtc(valueString)} symbolIsPrefix={true} />
-  if (valueIsSats && displayMode === DISPLAY_MODE_SATS)
-    return <BalanceComponent symbol={satSymbol} value={formatSats(valueString)} symbolIsPrefix={false} />
-
-  if (valueIsBtc && displayMode === DISPLAY_MODE_SATS)
-    return <BalanceComponent symbol={satSymbol} value={formatSats(btcToSats(valueString))} symbolIsPrefix={false} />
-  if (valueIsSats && displayMode === DISPLAY_MODE_BTC)
-    return <BalanceComponent symbol={btcSymbol} value={formatBtc(satsToBtc(valueString))} symbolIsPrefix={true} />
+  const balanceComponent = (() => {
+    if (displayMode === DISPLAY_MODE_HIDDEN) {
+      return (
+        <BalanceComponent
+          symbol={
+            <span className="d-inline-flex align-items-center text-muted">
+              <Sprite symbol="hide" width="1.2em" height="1.2em" className="ps-1" />
+            </span>
+          }
+          value={'*****'}
+          symbolIsPrefix={false}
+        />
+      )
+    }
+
+    if (typeof valueString !== 'string') {
+      console.warn('<Balance /> component expects string input')
+      return <BalanceComponent symbol="" value={valueString} symbolIsPrefix={false} />
+    }
+
+    // Treat integers as sats.
+    const valueIsSats = valueString === Number.parseInt(valueString).toString()
+    // Treat decimal numbers as btc.
+    const valueIsBtc = !valueIsSats && !Number.isNaN(Number.parseFloat(valueString)) && valueString.indexOf('.') > -1
+
+    const btcSymbol = <span style={{ paddingRight: '0.1em' }}>{'\u20BF'}</span>
+    const satSymbol = <Sprite symbol="sats" width="1.2em" height="1.2em" />
+
+    if (valueIsBtc && displayMode === DISPLAY_MODE_BTC)
+      return <BalanceComponent symbol={btcSymbol} value={formatBtc(valueString)} symbolIsPrefix={true} />
+    if (valueIsSats && displayMode === DISPLAY_MODE_SATS)
+      return <BalanceComponent symbol={satSymbol} value={formatSats(valueString)} symbolIsPrefix={false} />
+
+    if (valueIsBtc && displayMode === DISPLAY_MODE_SATS)
+      return <BalanceComponent symbol={satSymbol} value={formatSats(btcToSats(valueString))} symbolIsPrefix={false} />
+    if (valueIsSats && displayMode === DISPLAY_MODE_BTC)
+      return <BalanceComponent symbol={btcSymbol} value={formatBtc(satsToBtc(valueString))} symbolIsPrefix={true} />
+
+    console.warn('<Balance /> component cannot determine balance format')
+    return <BalanceComponent symbol={''} value={valueString} symbolIsPrefix={false} />
+  })()
+
+  if (!enableVisibilityToggle) {
+    return <>{balanceComponent}</>
+  }
 
-  console.warn('<Balance /> component cannot determine balance format')
-  return <BalanceComponent symbol={''} value={valueString} symbolIsPrefix={false} />
+  return (
+    <span onClick={toggleVisibility} style={{ cursor: 'pointer' }}>
+      {balanceComponent}
+    </span>
+  )
 }
diff --git a/src/components/CurrentWalletMagic.jsx b/src/components/CurrentWalletMagic.jsx
index 36d462a..1d9907f 100644
--- a/src/components/CurrentWalletMagic.jsx
+++ b/src/components/CurrentWalletMagic.jsx
@@ -26,7 +26,12 @@ const WalletHeader = ({ name, balance, unit, showBalance, loading }) => {
       )}
       {!loading && (
         <h2>
-          <Balance valueString={balance} convertToUnit={unit} showBalance={showBalance || false} />
+          <Balance
+            valueString={balance}
+            convertToUnit={unit}
+            showBalance={showBalance || false}
+            enableVisibilityToggle={false}
+          />
         </h2>
       )}
     </div>
diff --git a/src/components/Navbar.jsx b/src/components/Navbar.jsx
index 86118eb..5bfa04e 100644
--- a/src/components/Navbar.jsx
+++ b/src/components/Navbar.jsx
@@ -24,6 +24,7 @@ const WalletPreview = ({ wallet, walletInfo, unit, showBalance }) => {
               valueString={walletInfo.data.display.walletinfo.total_balance}
               convertToUnit={unit}
               showBalance={showBalance || false}
+              enableVisibilityToggle={false}
             />
           </div>
         ) : (
diff --git a/src/index.css b/src/index.css
index 9d29e5b..6f4fa89 100644
--- a/src/index.css
+++ b/src/index.css
@@ -502,6 +502,10 @@ h2 {
     center/1em auto no-repeat;
 }
 
+.placeholder {
+  background-color: var(--bs-gray-400);
+}
+
 /* Privacy Level Styles */
 
 .privacy-level-4 {
@@ -610,10 +614,6 @@ h2 {
   --bs-black: #000;
 }
 
-.placeholder {
-  background-color: #e9ecef;
-}
-
 :root[data-theme='dark'] .border-top,
 :root[data-theme='dark'] .border-bottom,
 :root[data-theme='dark'] .border-left,


Test patch:
diff --git a/src/components/Balance.test.jsx b/src/components/Balance.test.jsx
index 551c499..8c62461 100644
--- a/src/components/Balance.test.jsx
+++ b/src/components/Balance.test.jsx
@@ -1,4 +1,6 @@
 import React from 'react'
+import { act } from 'react-dom/test-utils'
+import user from '@testing-library/user-event'
 import { render, screen } from '../testUtils'
 import { BTC, SATS } from '../utils'
 
@@ -116,4 +118,70 @@ describe('<Balance />', () => {
     render(<Balance valueString={43000} convertToUnit={SATS} showBalance={true} />)
     expect(screen.getByText(`43000`)).toBeInTheDocument()
   })
+
+  it('should toggle visibility of initially hidden balance on click by default', () => {
+    render(<Balance valueString={`21`} convertToUnit={SATS} showBalance={false} />)
+    expect(screen.queryByText(`21`)).not.toBeInTheDocument()
+    expect(screen.getByText(`*****`)).toBeInTheDocument()
+
+    act(() => {
+      user.click(screen.getByText(`*****`))
+    })
+
+    expect(screen.getByText(`21`)).toBeInTheDocument()
+    expect(screen.queryByText(`*****`)).not.toBeInTheDocument()
+
+    act(() => {
+      user.click(screen.getByText(`21`))
+    })
+
+    expect(screen.queryByText(`21`)).not.toBeInTheDocument()
+    expect(screen.getByText(`*****`)).toBeInTheDocument()
+  })
+
+  it('should NOT toggle visibility of initially hidden balance on click when disabled via flag', () => {
+    render(<Balance valueString={`21`} convertToUnit={SATS} showBalance={false} enableVisibilityToggle={false} />)
+    expect(screen.queryByText(`21`)).not.toBeInTheDocument()
+    expect(screen.getByText(`*****`)).toBeInTheDocument()
+
+    act(() => {
+      user.click(screen.getByText(`*****`))
+    })
+
+    expect(screen.queryByText(`21`)).not.toBeInTheDocument()
+    expect(screen.getByText(`*****`)).toBeInTheDocument()
+  })
+
+  it('should NOT toggle visibility of initially visible balance on click by default', () => {
+    render(<Balance valueString={`21`} convertToUnit={SATS} showBalance={true} />)
+    expect(screen.getByText(`21`)).toBeInTheDocument()
+    expect(screen.queryByText(`*****`)).not.toBeInTheDocument()
+
+    act(() => {
+      user.click(screen.getByText(`21`))
+    })
+
+    expect(screen.getByText(`21`)).toBeInTheDocument()
+    expect(screen.queryByText(`*****`)).not.toBeInTheDocument()
+  })
+
+  it('should toggle visibility of initially visible balance on click when enabled via flag', () => {
+    render(<Balance valueString={`21`} convertToUnit={SATS} showBalance={true} enableVisibilityToggle={true} />)
+    expect(screen.getByText(`21`)).toBeInTheDocument()
+    expect(screen.queryByText(`*****`)).not.toBeInTheDocument()
+
+    act(() => {
+      user.click(screen.getByText(`21`))
+    })
+
+    expect(screen.queryByText(`21`)).not.toBeInTheDocument()
+    expect(screen.getByText(`*****`)).toBeInTheDocument()
+
+    act(() => {
+      user.click(screen.getByText(`*****`))
+    })
+
+    expect(screen.getByText(`21`)).toBeInTheDocument()
+    expect(screen.queryByText(`*****`)).not.toBeInTheDocument()
+  })
 })