feat: Implemented some Animo colors #54

Tommylans · 2022-12-23T13:09:08Z

closes #55

packages/toolbox-core/src/agent/config/adapters/ToolBoxConfigValidation.ts

packages/toolbox-ui/src/ToolboxApp.tsx

berendsliedrecht

Just one point mainly about how the theme is being used.

I will just comment it here again. We should never really do custom theme stuff within the code outside of the themes file.

Where we define the collars we should names like: backgroundOne, backgroundTwo, primaryOne, primaryTwo, secondaryOne, secondaryTwo, grayscaleOne, grayscaleTwo, info, success, etc.

Each color maps to something different in light and dark mode, with the exception of colors that MUST be the same in light and dark mode (e.g. grayscale).

If we map it like this we can avoid all the manual ternary expressions for the theme and make it a lot easier to create pages that are instantly compatible with light/dark mode.

berendsliedrecht · 2023-01-05T09:10:27Z

packages/toolbox-ui/src/components/generic/buttons/Buttons.tsx

+const useStyles = createStyles((theme) => ({
+  primary: {
+    backgroundColor: theme.colorScheme === 'dark' ? theme.colors.animoWhite[0] : theme.colors.animoBlack[0],
+    color: theme.colorScheme === 'dark' ? theme.colors.animoBlack[0] : theme.colors.animoWhite[0],
+
+    '&:hover': {
+      backgroundColor: theme.colorScheme === 'dark' ? theme.colors.animoWhite[0] : theme.colors.animoBlack[0],
+    },
+  },
+
+  secondary: {
+    backgroundColor: theme.colorScheme === 'dark' ? theme.colors.animoBlack[0] : theme.colors.animoWhite[0],
+    color: theme.colorScheme === 'dark' ? theme.colors.animoWhite[0] : theme.colors.animoBlack[0],
+
+    '&:hover': {
+      backgroundColor: theme.colorScheme === 'dark' ? theme.colors.animoBlack[1] : theme.colors.animoWhite[1],
+    },
+  },
+}))


Yeah I am not a fan of this. We have a theme which we can change based on the light / dark mode.

If we use names like theme.colors.backgroundOne / Two / etc we can just change the colours when the theme changes and not like this. We really must avoid this when possible as this will make code quite unreadable and annoying to fix later on.

The only thing we can do I think is making named colors without the color name and switch those in the color pallet. I'll look into it tomorrow.

Yeah that is what I mean. IMO specifying a color by name should only be done if it is the same color in different themes. And if we do that for all, why do we have themes?

berendsliedrecht · 2023-01-05T09:11:49Z

packages/toolbox-ui/src/components/generic/table/StatusBadge.tsx

+export const StatusBadge = (props: BadgeProps) => {
+  const { colorScheme } = useMantineTheme()
+
+  return <Badge variant={colorScheme === 'dark' ? 'light' : 'outline'} color="blue" {...props} />


Same here. I think we should really, with one or max two exceptions, never check the colorscheme within code here. If we want to display a different icon then I understand but we should just switch colors between light and dark mode and preferably nothing else.

Hmm this one will be a bit harder I think because we are changing the variant and this one isn't really switchable with the global color pallet. But we might be able to create a hook for it to change it between the 2. So maybe a hook that tells that we need more contrast with a specific colorscheme so just a yes or no return.

I don’t mean that we should change the variant based on the theme, I mean that we should NOT toggle anything other than the colors based on the light theme

Signed-off-by: Tom Lanser <[email protected]>

…us badge generic Signed-off-by: Tom Lanser <[email protected]>

Signed-off-by: Tom Lanser <[email protected]>

berendsliedrecht · 2023-01-09T10:31:39Z

@janrtvld Could you do a quick check for this PR? You will likely have some better things to say about the design / colors then I do :).

… avatar at the agent selection Signed-off-by: Tom Lanser <[email protected]>

…e the navbar Signed-off-by: Tom Lanser <[email protected]>

Tommylans · 2023-01-11T09:51:54Z

I made a lot of changes to the UI and also processed some of the feedback that @janrtvld gave.

The result:

Signed-off-by: Tom Lanser <[email protected]>

berendsliedrecht

Some very minor things. can be merged after resolution.

berendsliedrecht · 2023-01-11T10:50:34Z

packages/toolbox-electron/src/preload.js

@@ -31,7 +31,8 @@ contextBridge.exposeInMainWorld('nodeFetch', async (endpoint, request) => {

 const getConfigDirForPlatform = (platform, homeDir, appDataDir) => {
  switch (platform) {
-    case 'linux' || 'darwin':
+    case 'darwin':


You switched it back 👀 that's fine haha :).

Yeah it wasn't working and javascript doesn't support the || operator in a switch or it wasn't working as expected. Darwin didn't work with this switch so idk It's stranngeeee. So when I switched it back it started working again on my macbook.

packages/toolbox-ui/src/components/Loading.tsx

packages/toolbox-ui/src/components/connections/ConnectionsTable.tsx

packages/toolbox-ui/src/components/credentials/CredentialsTable.tsx

Signed-off-by: Tom Lanser <[email protected]>

berendsliedrecht · 2023-01-11T13:48:06Z

Can be merged @Tommylans if you want.

Tommylans force-pushed the feature/TBX-62 branch from 5ad9199 to 709bfdd Compare December 23, 2022 13:09

Tommylans force-pushed the feature/TBX-62 branch 2 times, most recently from 9f06430 to 3644dab Compare January 4, 2023 11:21

Tommylans marked this pull request as ready for review January 4, 2023 12:57

Tommylans requested a review from berendsliedrecht January 4, 2023 12:58

Tommylans force-pushed the feature/TBX-62 branch 2 times, most recently from 3cf309b to 226c506 Compare January 4, 2023 13:13

berendsliedrecht reviewed Jan 4, 2023

View reviewed changes

packages/toolbox-core/src/agent/config/adapters/ToolBoxConfigValidation.ts Outdated Show resolved Hide resolved

packages/toolbox-ui/src/ToolboxApp.tsx Outdated Show resolved Hide resolved

Tommylans mentioned this pull request Jan 4, 2023

Abstract Mantine theme to a separate file for easier theme management #55

Closed

berendsliedrecht requested changes Jan 5, 2023

View reviewed changes

Tommylans added 2 commits January 6, 2023 17:09

feat: Implemented some Animo colors

c0211de

Signed-off-by: Tom Lanser <[email protected]>

feat: Changed the color usage of the action buttons and made the stat…

02a5af6

…us badge generic Signed-off-by: Tom Lanser <[email protected]>

Tommylans force-pushed the feature/TBX-62 branch from 1e3365a to 02a5af6 Compare January 6, 2023 16:09

Tommylans marked this pull request as draft January 6, 2023 16:19

refactor: All of the screens are now supporting theme variables

77320ff

Signed-off-by: Tom Lanser <[email protected]>

Tommylans force-pushed the feature/TBX-62 branch from 96e59e5 to 77320ff Compare January 9, 2023 09:59

refactor: Splitted the different theme elements in their own file

f7cca14

Signed-off-by: Tom Lanser <[email protected]>

Tommylans marked this pull request as ready for review January 9, 2023 10:10

Tommylans requested a review from berendsliedrecht January 9, 2023 10:10

Merge branch 'main' into feature/TBX-62

c611dfd

Tommylans added 2 commits January 10, 2023 10:53

feat: Made some texts a bit brighter and changed the roundness of the…

bed4500

… avatar at the agent selection Signed-off-by: Tom Lanser <[email protected]>

feat: Added a empty state and changed some styling and the way we hav…

767205d

…e the navbar Signed-off-by: Tom Lanser <[email protected]>

fix: Fixed some linting errors

62eb493

Signed-off-by: Tom Lanser <[email protected]>

Tommylans force-pushed the feature/TBX-62 branch from 00cdb8d to 62eb493 Compare January 11, 2023 10:11

berendsliedrecht approved these changes Jan 11, 2023

View reviewed changes

Tommylans mentioned this pull request Jan 11, 2023

The textONE textTWO were supposed to be the shades. so it would be like text[0] -> textOne. textOne[0] is a bit double. #62

Open

refactor: Processed the PR comments

288a186

Signed-off-by: Tom Lanser <[email protected]>

Tommylans merged commit c91c4fd into main Jan 11, 2023

Tommylans deleted the feature/TBX-62 branch January 11, 2023 13:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Implemented some Animo colors #54

feat: Implemented some Animo colors #54

Tommylans commented Dec 23, 2022 •

edited

Loading

berendsliedrecht left a comment

berendsliedrecht Jan 5, 2023

Tommylans Jan 5, 2023

berendsliedrecht Jan 5, 2023

berendsliedrecht Jan 5, 2023

Tommylans Jan 5, 2023

berendsliedrecht Jan 5, 2023

berendsliedrecht commented Jan 9, 2023

Tommylans commented Jan 11, 2023

berendsliedrecht left a comment

berendsliedrecht Jan 11, 2023

Tommylans Jan 11, 2023

berendsliedrecht commented Jan 11, 2023

feat: Implemented some Animo colors #54

feat: Implemented some Animo colors #54

Conversation

Tommylans commented Dec 23, 2022 • edited Loading

berendsliedrecht 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

berendsliedrecht commented Jan 9, 2023

Tommylans commented Jan 11, 2023

berendsliedrecht left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

berendsliedrecht commented Jan 11, 2023

Tommylans commented Dec 23, 2022 •

edited

Loading