refactor: Disable other options if three categories are selected while creating a post #73
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
Hey @SanjeevKrSingh, Thank you for your quick update,
|
|
|
||
| return ( | ||
| <span | ||
| className={twMerge( | ||
| 'cursor-pointer rounded-3xl px-3 py-1 text-xs font-medium text-light-primary/80 dark:text-dark-primary/80', | ||
| selected ? selectedColor : defaultColor | ||
| selected ? selectedColor : defaultColor, | ||
| disabled && disabledColor | ||
| )} |
There was a problem hiding this comment.
disabled && disabledColor
I didn't get this logic, what is this? What are you trying to achieve? by this
Then you can simply add disabled ? disabledColor : defaultColor to the twMerge() argument.
#69 (comment)
Why did you chose not to do this way? any reason?
There was a problem hiding this comment.
I get the logic but it has its pitfalls. Using a ternary will always be preferable.
Read this article to see why you should avoid using logical AND for conditional rendering: https://kentcdodds.com/blog/use-ternaries-rather-than-and-and-in-jsx
There was a problem hiding this comment.
Hey @chinmaykunkikar @krishnaacharyaa here there is three state of the category
First is nomal state
second is selected state
and third is disabled state
If i do like this
disabled ? disabledColor : defaultColor
how to handle selected state we need to like this
disabled ? disabledColor : selected ? selectedColor : defaultColor
There was a problem hiding this comment.
@SanjeevKrSingh
disabled ? disabledColor : selected ? selectedColor : defaultColor
This seems reasonable to me.
But for better reading, we can split it to
disabled ? disabledColor : getSelectedColor(selected)
ORgetCategoryColor(disabled,selected)
InsidegetCategoryColorfunction, we can dodisabled ? disabledColor : getSelectedColor(selected)
I find 2 modular and precise, and easy to understand, if you have any better approaches, feel free to let us know :) @chinmaykunkikar kindly share your thoughts if you get free time.
| setFormData({ | ||
| ...formData, | ||
| categories: [...formData.categories, category], | ||
| categories: formData.categories.includes(category) ? formData.categories.filter((cat) => cat !== category) : [...formData.categories, category] | ||
| }); |
There was a problem hiding this comment.
We need to split this into understandable code, as already reviewed in the previous PR, kindly work on it.
| @@ -238,6 +235,7 @@ function AddBlog() { | |||
| {categories.map((category, index) => ( | |||
| <span key={`${category}-${index}`} onClick={() => handleCategoryClick(category)}> | |||
| <CategoryPill | |||
| disabled={isValidCategory(category)} | |||
| category={category} | |||
There was a problem hiding this comment.
Much better,
to something similar to this you can do in the above comment
|
@SanjeevKrSingh @krishnaacharyaa I just realised that we can further improve UX by adding a simple hint to the Before
We can simply add The final result will look like this: After
What do you think? |
|
We are closing this, as it is inactive and the author confirmed he isn't going to work on this |
ghost
deleted the
Summary
Disable other options if three categories are selected while creating a post
Description
To improve the current UX, disable the remaining categories when the user selects three categories while a new blog post is being created.
Images
Issue(s) Addressed
Closes #66
Prerequisites