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

Footer Link Component Bug #31

Closed
sounmind opened this issue Jun 11, 2024 · 1 comment · Fixed by #36
Closed

Footer Link Component Bug #31

sounmind opened this issue Jun 11, 2024 · 1 comment · Fixed by #36
Assignees
Labels
bug Something isn't working

Comments

@sounmind
Copy link
Member

sounmind commented Jun 11, 2024

@bhyun-kim 님 버그를 고쳐봅시다!
이 버그 때문에 다른 컴포넌트 코드가 실행되지 않고 있습니다.

Image
Image

왜 발생했고, 어떻게 하면 이런 버그를 방지할 수 있을까요?
그리고 이런 버그를 발견했을 때 어떤 과정으로 고치는지도 집중해보면 좋겠습니다.

@sounmind sounmind converted this from a draft issue Jun 11, 2024
@sounmind sounmind added the bug Something isn't working label Jun 11, 2024
@yolophg
Copy link
Contributor

yolophg commented Jun 11, 2024

추가로 버그 픽스에 힌트를 드리자면, 제가 suggestion 했던 코드를 그대로 복사하신 것 같은데, 기존에 이미 있는 constructor() 내로 옮기라는 의미였기에 constructor()를 다시 생성하실 필요는 없으십니다. constructor는 하나의 클래스 내에 하나만 존재할 수 있습니다 :)
덧붙여, 매번 this.shadowRoot.getElementById("link")를 호출해주기보다, 초기에 인스턴스 변수를 하나 만들어 한번만 link element에 저장해주는 것도 좋을 것 같다 라고 말씀을 드리며 아래 코드도 같이 suggestion 했었는데 이 뜻이, 매번 호출 -> 초기 constructor로 옮겨 변수 만들어서 거기에서 한 번만 호출이라는 의미였습니다!
제 설명이 좀 부족했을 수도 있겠다 싶네요😀

// 초기에 link element 인스턴스 변수 만들어서 저장하고 초기 값 설정
constructor() {
    this.linkElement = shadow.getElementById("link");
    this.updateLink(this.getAttribute("href"));
}

그리고, 해당 에러 수정 후, 에러 하나 더 뜰거에요. 왜냐면, attributeChangedCallback 내 updateLinkHref()라는 메소드는 정의되어있지 않거든요. updateLink() 메소드를 제가 첨에 updateLinkHref()로 네이밍했다가 수정했던거라, typo를 냈었네요. 그래서, 혼선을 드린 것 같습니다!😅
당연하게도, 하나의 메소드 네이밍은 통일 되어야하므로 병현님이 원하시는 네이밍으로 통일해주시면 되겠습니다!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
No open projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants