Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: user policy typo #61

Merged
merged 3 commits into from
Jan 15, 2025
Merged

Conversation

steven-yn
Copy link
Contributor

시점 이동 줄이기 (https://frontend-fundamentals.com/code/examples/user-policy.html) 내용을 읽다가 예제 코드에 누락된 부분이 있는것 같습니다.

'조건을 한 눈에 볼 수 있는 객체로 만들기' 문단에서 리팩터링된 예제 코드중,
policy 객체의 프로퍼티 admin 과 viewer 에 액세스 하기위한 코드를 추가했습니다

Copy link

vercel bot commented Jan 14, 2025

@steven-yn is attempting to deploy a commit to the Toss Team on Vercel.

A member of the Team first needs to authorize it.

@steven-yn steven-yn changed the title fix: missing property access fix: user policy typo Jan 14, 2025
@Seol-JY
Copy link
Contributor

Seol-JY commented Jan 14, 2025

How about updating the English version too? (en/code/examples/user-policy.md)

@Seol-JY
Copy link
Contributor

Seol-JY commented Jan 14, 2025

I'd like to suggest a correction regarding the disabled prop in code. Currently, it directly uses the permission values, but this creates an inverted logic:

function Page() {
  const user = useUser();
  const policy = {
    admin: { canInvite: true, canRead: true },
    viewer: { canInvite: false, canRead: true },
  };
  const userPolicy = policy[user.role];

  return (
    <div>
      {/* Current: Button is disabled when permission is true */}
      <Button disabled={userPolicy.canInvite}>Invite</Button>
      <Button disabled={userPolicy.canRead}>Read</Button>

      {/* Should be: Button is disabled when permission is false */}
      <Button disabled={!userPolicy.canInvite}>Invite</Button>
      <Button disabled={!userPolicy.canRead}>Read</Button>
    </div>
  );
}

@karinajjang
Copy link

how about this?

function Page() {
  const user = useUser();
  const policy = {
    admin: { canInvite: true, canRead: true },
    viewer: { canInvite: false, canRead: true },
  }[user.role];

  return (
    <div>
      <Button disabled={policy.canInvite}>Invite</Button>
      <Button disabled={policy.canRead}>Read</Button>
    </div>
  );
}

@Seol-JY
Copy link
Contributor

Seol-JY commented Jan 14, 2025

I think this issue is caused by this. #58

@steven-yn
Copy link
Contributor Author

steven-yn commented Jan 15, 2025

I think both of you are right, so I changed below

function Page() {
  const user = useUser();
  const policy = {
    admin: { canInvite: true, canRead: true },
    viewer: { canInvite: false, canRead: true },
  }[user.role];

  return (
    <div>
      <Button disabled={!policy.canInvite}>Invite</Button>
      <Button disabled={!policy.canRead}>Read</Button>
    </div>
  );
}

thanks @karinajjang @Seol-JY

Copy link
Collaborator

@milooy milooy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💯

@milooy milooy merged commit 1b7300a into toss:main Jan 15, 2025
1 check failed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants