fix: script error is caused by calling removeCheckedRows API(#1053)
#1052
Conversation
| @@ -371,7 +371,7 @@ describe('removeRow()', () => { | |||
| describe('removeCheckedRows()', () => { | |||
| beforeEach(() => { | |||
| // @ts-ignore | |||
| createGrid({ rowHeaders: ['_checked'] }); | |||
| createGrid({ rowHeaders: ['checkbox'] }); | |||
There was a problem hiding this comment.
언더바가 제거된 이유가 있나요?? 제 기억에.. 로우 헤더 설정할 때 값이 문자열일 때 컬럼명을 그대로 사용했던 것 같아서요 🤔
There was a problem hiding this comment.
옵션명이 처음부터 잘못 들어갔어요. 튜토리얼에서도 checkbox로 가이드하고 있습니다~!
| for (let index = 0, end = rawData.length; index < end; index += 1) { | ||
| const targetIndex = targetIndexes[index]; | ||
| data.viewData.splice(targetIndex, 1, viewData[index]); | ||
| data.rawData.splice(targetIndex, 1, rawData[index]); | ||
| } |
There was a problem hiding this comment.
데이터를 한개씩 추가해주다보면 데이터가 섞일 것 같은데 순서 보장이 되는지 궁금합니다!
There was a problem hiding this comment.
data.viewData랑 viewData는 다른 데이터라서 상관없을 것 같습니다~!
| @@ -38,12 +38,13 @@ function calculateRange( | |||
| let start = findIndexByPosition(offsets, scrollPos); | |||
| let end = findIndexByPosition(offsets, scrollPos + totalSize) + 1; | |||
| const { filteredRawData, sortState, pageRowRange } = data; | |||
| const dataLength = filteredRawData.length; | |||
There was a problem hiding this comment.
filteredRawData가 null이나 undefined 처리되는 경우는 없나요~?
There was a problem hiding this comment.
넵~!필터 조건이 없는 경우는 rawData와 항상 동일하기 때문에 없습니다
| cy.getCell(0, 'name').should('exist'); | ||
| cy.getCell(1, 'name').should('not.exist'); | ||
|
|
||
| cy.getRowHeaderCell(0, '_checked') |
There was a problem hiding this comment.
prettier 버전을 올리면 이런것들이 한줄로 표현이 될텐데..언제 한번 날을 잡고 전체적으로 올려도 좋을 것 같네요
There was a problem hiding this comment.
넵 안그래도 하루 잡고 cypress랑 storybook 도 전부 같이 올릴 예정입니다.
| return; | ||
| } | ||
|
|
||
| Object.keys(prevRowSpanMap).forEach(columnName => { |
There was a problem hiding this comment.
생각해보니까..저희 forEachObject라는 메서드가 있었는데.. 전체를 찾아보니까 별로 사용하는곳이 없네요///
There was a problem hiding this comment.
사실 이런 케이스는 성능은 크게 상관없을 것 같은데 아무래도 Object.keys를 사용하는게 더 빠르기도 하고 이 함수를 써도 코드가 많이 줄거나 그렇지 않기 때문에 더 안 쓰게 된 것 같긴하네요. forEachObject 내부에서 Object.keys를 좀더 편하게 쓰도록 랩핑하는 정도로 변경하면 될 것같은데 그럼 별로 이 유틸함수의 의미가 없어지기도 하고 추후 제거해도 될 것 같네요
js87zz
changed the title
removeCheckedRows APIremoveCheckedRows API(#1053)
js87zz
force-pushed
the
fix/removeCheckedRows-script-error
branch
from
a03b074
to
88168e2
Compare
Please check if the PR fulfills these requirements
fix #xxx[,#xxx], where "xxx" is the issue number)Description
removeCheckedRowsAPI #1053)Thank you for your contribution to TOAST UI product. 🎉 😘 ✨