Paul/asset gradient #4702

paullinator · 2024-01-11T01:08:33Z

CHANGELOG

Does this branch warrant an entry to the CHANGELOG?

Yes
No

Dependencies

none

Requirements

If you have made any visual changes to the GUI. Make sure you have:

Tested on iOS device
Tested on Android device
Tested on small-screen device (iPod Touch)
Tested on large-screen device (tablet)

To see the specific tasks where the Asana app for GitHub is being used, see below:
- https://app.asana.com/0/0/1206303846046806

src/types/Theme.ts

samholmes · 2024-01-12T20:33:45Z

src/types/Theme.ts

-  // The accent color will override the dot color when it is present.
-  // To change this behavior, use 'keep' to preserve the dot color or
-  // 'drop' to remove the dot completely:
-  accent?: 'keep' | 'drop'


Per our discussion, it seems the better approach to lending the theme files the control it may need to respond to certain scene changes is to have background declarations per scene assetBackground, marketsBackground, etc. Each declaration can declare if it wants to use some values provided by the scene such as asset color or some other variables that the scene may procure. If it doesn't make sense that the scene can procure such a value, then the Theme.ts file should parameterize the different background fields with respect to their capabilities; this can be done with a generic prop to ThemeDot.

src/components/ui4/DotsBackground.tsx

src/types/Theme.ts

src/util/utils.ts

samholmes · 2024-01-12T21:03:52Z

src/components/scenes/TransactionListScene.tsx

+    const scaledColor = multiplyHexColor(iconColor, theme.assetBackgroundColorScale)
+    backgroundColors[0] = scaledColor


This scaling seems like a change rather than a refactor.

The scale was hard coded and now is in the theme. The old scaling was based on alpha because there was a flat black background underneath. Now the background is only the gradient so we can't apply an alpha to it.

Ah, I see, so it's a way to achieve the ability to darken the background color from the theme file (because it's currently set to #00000000, transparent black). Now we can darken or lighten the value...wait, we can lighten the value if we pass a scale > 1, so clipping is still an issue that should be handled in the "darkenHexColor" function. I'd argue "scaleHexColor" might be a better name because darkening isn't the only use.

src/components/ui4/DotsBackground.tsx

src/components/scenes/RequestScene.tsx

samholmes · 2024-01-12T21:10:00Z

src/hooks/useIconColor.ts

+import { EdgeAsset } from '../types/types'
+import { getCurrencyIconUris } from '../util/CdnUris'
+
+export const useIconColor = (edgeAsset: EdgeAsset): string | undefined => {


Why aren't we using this this TransactionListScene for consistency?

Because we'd be grabbing the icon color twice. The TxListScene already loads the icon for other purposes so we get the color then.

My suggestion was to gut the icon grab out of the CyptoIconUi4 in favor of this hook. But, no worries if not.

This gives a more flexible API to modify any parameters of the dots on any screen.

samholmes

I'm approving this as I think it's ready. The only concern I have is with the hex color function which doesn't clip the color values.

src/types/Theme.ts

samholmes · 2024-01-13T02:14:42Z

src/util/utils.ts

+ * @param scaleFactor 0-1 with 0 being black, 1 is unchanged
+ * @returns darkened hex color string
+ */
+export const darkenHexColor = (hexColor: string, scaleFactor: number): string => {


Ah this makes a lot more sense. Can we keep the alpha channel for the hexColor?

src/components/ui4/DotsBackground.tsx

samholmes · 2024-01-13T02:23:34Z

src/components/scenes/TransactionListScene.tsx

+    const scaledColor = multiplyHexColor(iconColor, theme.assetBackgroundColorScale)
+    backgroundColors[0] = scaledColor


Ah, I see, so it's a way to achieve the ability to darken the background color from the theme file (because it's currently set to #00000000, transparent black). Now we can darken or lighten the value...wait, we can lighten the value if we pass a scale > 1, so clipping is still an issue that should be handled in the "darkenHexColor" function. I'd argue "scaleHexColor" might be a better name because darkening isn't the only use.

src/types/Theme.ts

src/components/ui4/DotsBackground.tsx

samholmes · 2024-01-13T02:37:54Z

src/hooks/useIconColor.ts

+import { EdgeAsset } from '../types/types'
+import { getCurrencyIconUris } from '../util/CdnUris'
+
+export const useIconColor = (edgeAsset: EdgeAsset): string | undefined => {


My suggestion was to gut the icon grab out of the CyptoIconUi4 in favor of this hook. But, no worries if not.

paullinator · 2024-01-13T06:34:38Z

/rebase

This let's specific scenes override the background with a gradient of their choice. TransactionList overrides by taking the iconColor and scaling it down

This matches more nicely with all the variable/identifier names.

paullinator force-pushed the paul/assetGradient branch 2 times, most recently from a29ebbd to 2e80d7c Compare January 12, 2024 09:25

samholmes requested changes Jan 12, 2024

View reviewed changes

paullinator force-pushed the paul/assetGradient branch from 2e80d7c to 14c53bb Compare January 13, 2024 01:34

paullinator added 3 commits January 12, 2024 18:20

Rename theme.background -> backgroundDots

4bd2c0f

Change accentColor to overrideDots

b55dc07

This gives a more flexible API to modify any parameters of the dots on any screen.

Reimplement background color as a gradient

085bdd9

paullinator force-pushed the paul/assetGradient branch from 14c53bb to f0ca7e7 Compare January 13, 2024 02:20

samholmes approved these changes Jan 13, 2024

View reviewed changes

paullinator and others added 3 commits January 13, 2024 06:35

Add background override

1960869

This let's specific scenes override the background with a gradient of their choice. TransactionList overrides by taking the iconColor and scaling it down

Add iconcolor gradient to send, request, earn, and txdetails

aada9f5

Rename UpdateDots to OverrideDots

a90214b

This matches more nicely with all the variable/identifier names.

github-actions bot force-pushed the paul/assetGradient branch from bb018f7 to a90214b Compare January 13, 2024 06:35

paullinator enabled auto-merge January 13, 2024 06:37

paullinator disabled auto-merge January 13, 2024 15:36

paullinator merged commit 39af328 into develop Jan 13, 2024
1 check passed

paullinator deleted the paul/assetGradient branch January 13, 2024 15:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Paul/asset gradient #4702

Paul/asset gradient #4702

paullinator commented Jan 11, 2024 •

edited

Loading

samholmes Jan 12, 2024

samholmes Jan 12, 2024

paullinator Jan 13, 2024

samholmes Jan 13, 2024

samholmes Jan 12, 2024

paullinator Jan 13, 2024

samholmes Jan 13, 2024

samholmes left a comment

samholmes Jan 13, 2024

samholmes Jan 13, 2024

samholmes Jan 13, 2024

paullinator commented Jan 13, 2024

		const scaledColor = multiplyHexColor(iconColor, theme.assetBackgroundColorScale)
		backgroundColors[0] = scaledColor

Paul/asset gradient #4702

Paul/asset gradient #4702

Conversation

paullinator commented Jan 11, 2024 • edited Loading

CHANGELOG

Dependencies

Requirements

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

samholmes left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

paullinator commented Jan 13, 2024

paullinator commented Jan 11, 2024 •

edited

Loading