dooboolab-community/dooboo-ui

Remove some chained ternaries

yujonglee opened this issue · 6 comments

yujonglee commented

I like chained ternaries and I love how they used in this project.
But I think sometimes chained ternaries are not the best option. (depends on logic)

https://github.com/dooboolab/dooboo-ui/blob/8187fcb6c8813f84561e9da667bde058b7fd1de0/main/Styled/StyledComponents.tsx#L29-L46

For example, I think above code snippet might be more readable and sustainable when written like this :

{
...

  background-color: ${({theme, type, outlined, disabled}) => {
    if (disabled) return undefined; 

    if (outlined) return theme.background;

    return theme[type ?? 'primary'];
  }};
}

Please let me know your opinion on this. 😄

hyochan commented

I definitely agree with this point of view! Please try the PR if you can.

  • 👍1
  • 🚀1
yujonglee commented

This refactoring can be done many places in dooboo-ui.

I only did some. There's lot left. So anyone can contribute by doing it.

  • 👍1
hyochan commented

Yes, #46 was just awesome!

Einere commented

@hyochan @yujong-lee
i think this issue has to be closed.. 😉

yujonglee commented

@Einere I'm not sure whether all the codes related to this have been fixed, but most of them seem to have been modified, so I will close it for now.

Thanks.

Einere commented

@yujong-lee
Ah, I didn't aware of that.
I just commented because related PR is merged. sorry.. 😅

  • 👍1