feat: add review item component #37
Conversation
| } | ||
|
|
||
| // initial render in here to ensure the element is fully connected to the DOM. | ||
| connectedCallback() { |
There was a problem hiding this comment.
connectedCallback을 사용하면 더 안전하게 render할 수 있겠군요! 참고하겠습니다!
| return ["text", "name", "img"]; | ||
| } | ||
|
|
||
| attributeChangedCallback(attrName, oldVal, newVal) { |
There was a problem hiding this comment.
혹시 constructor 나 connectedCallback이 아닌 observedAttributes과 attributeChangedCallback을 사용에 render method를 넣었을 때 얻을 수 있는 이점을 알 수 있을까요?
There was a problem hiding this comment.
해당 질문이 혹시, 동적 변경 관련 메소드를 사용한 것에 대한 질문일까요? 아니면, constructor/connectedCallback 말고, attributeChangedCallback에서 render()를 호출했을 때의 차이에 대해 질문하신걸까요? 아니면 둘 다 인걸까요?😄
전자의 대해 답변 드리자면, 첫 의도는 일단, 추후 확장성이나 유연성을 혹시라도 고려하는게 좋지 않을까라는 생각과, 사실 동적인게 더 익숙하고 편안해서 일단은 제가 그렇게 만들고 싶어서 작성을 해본 것도 있는 것 같습니다. 😀
근데, 동시에 한편으론, 완전 정적으로 유지보수가 될 것이고, 해당 컴포넌트가 추가적인 기능을 하게 될 가능성이 없을 듯 하여, 코드 간소화를 위해서라도 당장 불필요한 동적 변경 기능을 하는 메소드들은 제거를 하려했던 상태입니다...! 한 번 더 이 부분에 대해 짚어주셔서 감사합니다 :) 제거하고 다시 푸쉬할 예정입니다!
후자의 경우에 대해 답변 드리자면, constructor/connectedCallback에서 호출한 render(저의 경우에는 connectedCallback에서 호출했습니다)와 attributedChangedCallback에서 호출한 render의 의미는 완전히 다르게 의도해서 작성했습니다!
- connectedCallback에서 호출한 render : 엘리먼트가 DOM에 추가된 이후에 초기 렌더링을 위함입니다.
- attributeChangedCallback에서 호출한 render : 동적으로 구독된 attribute 중 하나가 변경되는 사항이 있을 때, 재렌더링을 해주기 위함입니다.
하지만, 결론적으로 동적 변경 기능을 하는 해당 메소드를 제거할 예정이라, render()함수를 포함하고 있던 불필요했던 메소드 또한 사라질테니, 최종적으로 유지보수에 더 좋은 방향일거라고 판단됩니다. 질문해주셔서 감사합니다 :)
There was a problem hiding this comment.
상세한 답변 감사드립니다! 지금 제 질문을 다시 보니 충분히 구체적으로 질문드리지 않아 오해의 소지가 있고 애매모호한 점이 많네요...ㅎㅎ 우선 질문은 전자에 대한 것 하나입니다! 저도 비슷한 생각으로 확장성을 고려해 동적 변경을 고려하여 개발했다가 전에 달레님께서 정적 유지보수로만 진행할 거라고 짚어주신 기억이 있고 테크리드님의 피드백도 있어서 관련 메서드를 뺐습니다! 혹시나 Helena님께서도 비숫한 생각으로 개발하신 것 같아 여쭤봤습니다! 부족한 질문이었음에도 불구하고 상세히 설명해주셔서 감사드립니다!
There was a problem hiding this comment.
좋습니다, 어제 미팅에서도 각자 스타일대로 컴포넌트를 작성 중이었던 점 잠깐 언급됐었는데 요부분을 어느정도는 통일감을 가져가는 것이 유지보수 측면에서도 그렇고 저희가 당장 협업하면서도 혼란이 적을 것 같습니다.
이 부분은 제가 진행토록 하겠습니다, #44 에 이번 이터레이션 이슈로 만들어두었습니다!
yolophg
force-pushed
the
helena/review-item-component
branch
from
e46d5cf
to
35ad673
Compare
| return this.getAttribute("img") || ""; | ||
| } | ||
|
|
||
| render() { |
| <div class="review-item"> | ||
| <div class="review-img-text-container"> | ||
| <div class="review-img"> | ||
| <img src="${this.img}" alt="Reviewer"> | ||
| </div> | ||
| <div class="review-text">${this.text}</div> | ||
| </div> | ||
| <div class="review-name">${this.name}</div> | ||
| </div> |
There was a problem hiding this comment.
이 부분 HTML 마크업을 다음과 같이 좀 더 sementic하게 하면 좀 더 좋지 않을까 생각이 들었습니다. (머지된 된 PR에 이렇게 의견을 드려서 죄송합니다 ㅠㅠ)
<figure>
<blockquote>
당신이 언젠가 죽을 것이라는 사실을 기억하는 것이야말로 무언가 잃을지도
모른다는 두려움에 갖히는 것을 피할 수 있는, 제가 아는 가장 최고의
방법입니다.
</blockquote>
<figcaption>
<cite>스티브 잡스</cite>
</figcaption>
</figure>
No description provided.