Updating Changes acc. to #788 #1787

Hindu-Priya · 2024-05-05T14:39:55Z

Hi there, I have updated colors acc. to the top ones in the new design system styleguide tags, as per the dark mode theme.

changeset-bot · 2024-05-05T14:39:58Z

⚠️ No Changeset found

Latest commit: 5b8e246

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

dbradham

Looks good to me

momaqbol · 2024-05-05T17:18:32Z

packages/UI/src/styles/theme.ts

@@ -8,7 +8,7 @@ const theme = {
  },
  colors: {
    BLUE: '#3A7CA5',
-    BLUE_100: '#75A3C0',
+    BLUE_100: '#7A87C7',


changes to BLUE_100, YELLOW_100, ORANGE_100, SUCCESS_100 and this file as a whole need to be reviewed by design team before merging

Thanks Mohammed.

@Nouri-Anouar can you take a look at the changes here, or let us know if there is someone else who can help? Thanks!

@dbradham it appears this file is outdated and does not align with our design system.

Color Pallete

@Enjoy2Live can we have colors from the design tokens here?

@Enjoy2Live @Nouri-Anouar this seems out of scope for Hindu's change. Neither the old color, nor the color she is updating to are in the current design system you shared here.

CC: @JulieMass

@dbradham true and for that I'd recommend your team to use tailwind for cases like these because anything related to styled components is considered outdated/deprecated, for the now I think you should use the token Blue-blue-100 as tailwind className.
Example: bg-Blue-blue-100 applies #75A3C0 as background color

@Nouri-Anouar looking at tokens studio I'm still seeing Blue-blue-100 color token while it's not showing in the color pallete you linked, make sure to update either to match the other so we avoid confusions like these.

Just to give a bit of context for everyone, the color names have been updated as follows:

Light Blue -> Uranus.

Blue -> Neptune.

Yellow -> Saturn.

Orange -> Jupiter.

Additionally, there are additional values compared to this file ranging from 50 to 900. For dark mode, we're utilizing color values of 300, while 500 for light mode (ex: Color: Saturn 500, Mode: Light).

I believe we should use their respective TailwindCSS classes instead @Enjoy2Live?
I opened a PR for this fix #1791

thanks for the PR Anouar, this makes a ton of sense now.

fix(revert changes for tailwind on idea card tag)

momaqbol · 2024-06-07T09:22:03Z

apps/ideaspace/src/components/common/IdeaCard/IdeaCardTag.js

+    if (status === 'submitted') {
      setTag('submitted');
-      setTagColor(theme.colors.LIGHT_BLUE_100);
-    } else if (status == 'applying') {
+      setTagColor('--light-blue-lightblue-100');
+    } else if (status === 'applying') {
      setTag('in-review');
-      setTagColor(theme.colors.YELLOW_100);
-    } else if (status == 'approved') {
+      setTagColor('--yellow-yellow-100');
+    } else if (status === 'approved') {
      setTag('approved');
-      setTagColor(theme.colors.SUCCESS_100);
-    } else if (status == 'archived') {
+      setTagColor('--success-success-100');
+    } else if (status === 'archived') {
      setTag('archived');
-      setTagColor(theme.colors.BLUE_100);
+      setTagColor('--blue-blue-100');
    } else {
      setTag('workshopping');
-      setTagColor(theme.colors.ORANGE_100);
+      setTagColor('--orange-orange-100');


a more maintainable and readable way to handle this is by using a key value pair of [status]: 'tailwind-class'

const tagColor = { applying: 'bg-yellow-yellow-100' . . . } return <myDiv className={tagColor[status]} />

then you won't need most of this logic and useEffect

Hello @Enjoy2Live, I tried this approach, and I can't seem to access Tailwind CSS colors. Perhaps there is something missing, for not being able to apply Tailwind CSS classes correctly.

Hello @Hindu-Priya , you said you tried this approach but I'm not seeing the commit for your try.

can you please make a commit with my suggestion and push it so I can take a look?

Hello @Enjoy2Live, I just committed my attempt based on your suggestion. Please take a look when you get a chance.

momaqbol · 2024-06-16T03:53:26Z

I just merged the PR that should fix the colors to master, let me know if you face issues going forward

momaqbol

This is pretty much approved but please go through my requested changes and fix them before merging.

@dbradham it seems like there's duplicated work here, we've had a shared tag component already implemented some time ago. it's going to save you lots of time once you use as changes there are reflected on your app and the rest of the apps.

link to tag component stories
link to tag component file

momaqbol · 2024-06-20T03:37:36Z

apps/app/pages/_app.js

@@ -1,6 +1,10 @@
 import Head from 'next/head';
+import '../../../packages/tailwind-constructor/output/core.css';


this is not needed as we're already importing tailwind.css in this file.

Earlier, I encountered an error indicating that CSS should be included globally. This occurred when I didn't use the Tailwind class "bg-LightBlue-lightblue-100" in this manner. In the latest commit, I removed the comments and made minor changes. Thanks!

momaqbol · 2024-06-20T03:38:03Z

apps/ideaspace/src/components/common/IdeaCard/IdeaCardTag.js

@@ -1,31 +1,43 @@
 import React, { useState } from 'react';
-import theme from '@devlaunchers/components/styles/theme';
+// import theme from '@devlaunchers/components/styles/theme';


let's remove this instead of commenting

momaqbol · 2024-06-20T03:38:57Z

apps/ideaspace/src/components/common/IdeaCard/IdeaCardTag.js

+  const tagColor = {
+    submitted: 'bg-light-blue-lightblue-100',
+    'in-review': 'bg-yellow-yellow-100',
+    approved: 'bg-success-success-100',
+    archieved: 'bg-blue-blue-100',
+    workshopping: 'bg-orange-orange-100',
+  };


these should be typed like this:

const tagColor = { submitted: 'bg-LightBlue-lightblue-100', 'in-review': 'bg-Yellow-yellow-100', approved: 'bg-Success-success-100', archieved: 'bg-Blue-blue-100', workshopping: 'bg-Orange-orange-100', };

I fixed it in the last commit so you just need to pull

momaqbol · 2024-06-20T03:40:36Z

apps/ideaspace/src/components/common/IdeaCard/IdeaCardTag.js

+  //   }
+  // }, [status]);
+
+  return <StatuBox backgroundColor={tagColor[status]}> {status} </StatuBox>;


you should substitute backgroundColor prop with className because that's how tailwind works.

I fixed it in the last commit so you just need to pull

Hindu-Priya · 2024-06-23T22:22:41Z

Hi there, I made all the necessary changes. Please look at the latest commit, when you get a chance.

dbradham · 2024-06-24T04:00:35Z

Hi there, I made all the necessary changes. Please look at the latest commit, when you get a chance.

CC: @Enjoy2Live

momaqbol · 2024-06-25T21:05:23Z

apps/ideaspace/src/components/common/IdeaCard/IdeaCardTag.js

-  const [tag, setTag] = useState('');
-  // const [tagColor, setTagColor] = useState('');
+  if (status == 'applying') {
+    status = 'in-review';


@Hindu-Priya
props are immuatable so you can't assign different values to it if I'm not mistaken.

other than that this PR is approved by me

@Enjoy2Live that may be true in TypeScript, but I don't believe the same applies to JavaScript. Source: https://www.reddit.com/r/reactjs/comments/14howx9/why_im_able_to_change_props_value_if_props_are/?rdt=59264

it's not a typescript or javascript thing, it's a react thing.

in react you can't change props this way as props are immutable

Don’t try to “change props”. When you need to respond to the user input (like changing the selected color), you will need to “set state”, which you can learn about in State: A Component’s Memory.

Source: https://react.dev/learn/passing-props-to-a-component#how-props-change-over-time

@Enjoy2Live, @dbradham
I made a new commit, ensuring props immutability in the component. Thanks!

momaqbol

Now it's at a much better state but let's make sure we remove this file at some point and import the global tag component to ensure consistency and smaller codebase.

CC @dbradham

Approved!

P.S. this PR is going to master and I'm not sure if it's intended

Updating Changes acc. to #788

dbb90c0

Hindu-Priya requested review from a team as code owners May 5, 2024 14:39

dbradham approved these changes May 5, 2024

View reviewed changes

momaqbol requested changes May 5, 2024

View reviewed changes

Hindu-Priya and others added 3 commits June 2, 2024 11:28

Imported Tailwind CSS into IdeaCardTag

3344d68

fix(revert changes for tailwind on idea card tag)

1923da9

Merge pull request #1808 from dev-launchers/ideaspace/ideaTagColors

2f301a1

fix(revert changes for tailwind on idea card tag)

momaqbol reviewed Jun 7, 2024

View reviewed changes

Merge branch 'master' into UpdatingColors

15d5f4d

Trying to style tags with TailWind CSS

08d09b0

Hindu-Priya requested a review from pyxld-kris as a code owner June 20, 2024 03:14

fix tokens class case

932902c

momaqbol requested changes Jun 20, 2024

View reviewed changes

Styled Tags with Tailwind CSS

d4c8b1e

momaqbol reviewed Jun 25, 2024

View reviewed changes

Removed direct modifications to props.

5b8e246

momaqbol approved these changes Jun 27, 2024

View reviewed changes

dbradham merged commit 982ce6d into master Jun 29, 2024
2 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Updating Changes acc. to #788 #1787

Updating Changes acc. to #788 #1787

Hindu-Priya commented May 5, 2024 •

edited

Loading

changeset-bot bot commented May 5, 2024 •

edited

Loading

dbradham left a comment

momaqbol May 5, 2024

dbradham May 5, 2024

Nouri-Anouar May 7, 2024 •

edited

Loading

dbradham May 7, 2024

momaqbol May 8, 2024 •

edited

Loading

Nouri-Anouar May 11, 2024 •

edited

Loading

momaqbol May 14, 2024

momaqbol Jun 7, 2024 •

edited

Loading

Hindu-Priya Jun 19, 2024

momaqbol Jun 19, 2024

Hindu-Priya Jun 20, 2024

momaqbol commented Jun 16, 2024

momaqbol left a comment •

edited

Loading

momaqbol Jun 20, 2024

Hindu-Priya Jun 23, 2024

momaqbol Jun 20, 2024

momaqbol Jun 20, 2024

momaqbol Jun 20, 2024

Hindu-Priya commented Jun 23, 2024

dbradham commented Jun 24, 2024

momaqbol Jun 25, 2024

momaqbol Jun 25, 2024

dbradham Jun 26, 2024

momaqbol Jun 27, 2024

Hindu-Priya Jun 27, 2024 •

edited

Loading

momaqbol left a comment •

edited

Loading

		@@ -1,6 +1,10 @@
		import Head from 'next/head';
		import '../../../packages/tailwind-constructor/output/core.css';

Updating Changes acc. to #788 #1787

Updating Changes acc. to #788 #1787

Conversation

Hindu-Priya commented May 5, 2024 • edited Loading

changeset-bot bot commented May 5, 2024 • edited Loading

⚠️ No Changeset found

dbradham left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Nouri-Anouar May 7, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

momaqbol May 8, 2024 • edited Loading

Choose a reason for hiding this comment

Nouri-Anouar May 11, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

momaqbol Jun 7, 2024 • edited Loading

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

momaqbol commented Jun 16, 2024

momaqbol left a comment • edited Loading

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

Hindu-Priya commented Jun 23, 2024

dbradham commented Jun 24, 2024

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

Hindu-Priya Jun 27, 2024 • edited Loading

Choose a reason for hiding this comment

momaqbol left a comment • edited Loading

Choose a reason for hiding this comment

Hindu-Priya commented May 5, 2024 •

edited

Loading

changeset-bot bot commented May 5, 2024 •

edited

Loading

Nouri-Anouar May 7, 2024 •

edited

Loading

momaqbol May 8, 2024 •

edited

Loading

Nouri-Anouar May 11, 2024 •

edited

Loading

momaqbol Jun 7, 2024 •

edited

Loading

momaqbol left a comment •

edited

Loading

Hindu-Priya Jun 27, 2024 •

edited

Loading

momaqbol left a comment •

edited

Loading