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.

Sign up for GitHub

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

Jump to bottom

fix: Remove empty values from select filter (#1245) #1267

Merged
merged 3 commits into from Apr 23, 2021
Merged

fix: Remove empty values from select filter (#1245) #1267

merged 3 commits into from Apr 23, 2021

Conversation

ryum91
Copy link
Contributor

@ryum91 ryum91 commented Feb 11, 2021

Please check if the PR fulfills these requirements

  • It's submitted to right branch according to our branching model
  • It's right issue type on title
  • When resolving a specific issue, it's referenced in the PR's title (e.g. fix #xxx[,#xxx], where "xxx" is the issue number)
  • The commit message follows our guidelines
  • Tests for the changes have been added (for bug fixes/features)
  • Docs have been added/updated (for bug fixes/features)
  • It does not introduce a breaking change or has description for the breaking change

Description

Thank you for your contribution to TOAST UI product. 🎉 😘 ✨

ryum91
ryum91 commented Feb 11, 2021
View reviewed changes
packages/toast-ui.grid/src/query/data.ts
columnName,
targetData.map((data) => ({
...data,
[columnName]: isNil(data[columnName]) ? '' : data[columnName],
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Data에 null, '' 값 등 Nil 값이 여러개가 들어가있는 경우
SelectAll 체크 후, Empty Value Row 체크 해제를 하려면 Nil Row Count 만큼 눌러야 체크 해제가 되고 있어서
uniqColumnData 처리에 추가 필터링을 해주었습니다

js87zz
js87zz reviewed Mar 3, 2021
View reviewed changes
packages/toast-ui.grid/src/dispatch/filter.ts Outdated
Comment on lines 127 to 129
const validState = state.filter((item) => String(item.value).length);

if (type !== 'select' && !validState.length) {
Copy link
Contributor

@js87zz js87zz Mar 3, 2021 •
edited

Choose a reason for hiding this comment

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

이 체크 코드는 유지되어야겠네요~! 이 코드가 없으면 빈 텍스트를 입력(텍스트 필터인 경우)했을때 필터가 해제되지 않네요ㅎㅎ

@js87zz
Copy link
Contributor

js87zz commented Mar 3, 2021

@ryum91
작업해주신줄 모르고 있었네요. 리뷰가 너무너무 늦었군요;;😓
동작도 잘되고 딱히 이견없습니다. :)
다만 filter.spec.ts에 관련 동작 테스트(UI 시뮬레이션, 테스트의 invokeFilter API)로 추가만되면 될 것 같습니다!

@stale
Copy link

stale bot commented Apr 18, 2021

This issue has been automatically marked as inactive because there hasn’t been much going on it lately. It is going to be closed after 7 days. Thanks!

@stale stale bot added the inactive label Apr 18, 2021
@js87zz js87zz removed the inactive label Apr 21, 2021
@js87zz
Copy link
Contributor

js87zz commented Apr 23, 2021

@ryum91
이 PR 제가 마무리 작업하고 머지하겠습니다. 감사합니다 :)

@js87zz js87zz merged commit 646211c into nhn:master Apr 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@js87zz js87zz js87zz approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Select filter has empty options
2 participants
@ryum91 @js87zz