Added searchable filters in Ho_dan page #212
Conversation
There was a problem hiding this comment.
@dshongphuc Thanks for your hard work. I can see it in my local. However, there are few points we need to change before I can approve your PR:
- Code convention: CSS and JS - please take a look at my comments.
- We should have no search when select "status"
- Package-lock.json is totally changed, can you validate it, please?
- Translate choices.js
- No result found => Không tìm thấy kết quả
.env.sample
Outdated
| @@ -1,4 +1,5 @@ | |||
| DB_NAME=cuuhomientrung | |||
| DB_USER=postgres | |||
There was a problem hiding this comment.
@dshongphuc I don't see any reason we have to include the change of configuration example in this pull request. Can you exclude it?
| .form-control { | ||
| padding-left: .25rem; | ||
| padding-right: .25rem; | ||
| margin-bottom: .25rem; | ||
| } | ||
|
|
||
| .choices{ | ||
| position: relative; | ||
| &:focus{ outline: 1px solid #9fcfff!important; } |
There was a problem hiding this comment.
@dshongphuc Can you break it down to a new line here please? It should be
&:focus {
outline: 1px solid #9fcfff!important;
}
| .form-control { | ||
| padding-left: .25rem; | ||
| padding-right: .25rem; | ||
| margin-bottom: .25rem; | ||
| } | ||
|
|
||
| .choices{ |
There was a problem hiding this comment.
Can you add a space before curlybraces please? it should be .choices {
| .choices{ | ||
| position: relative; | ||
| &:focus{ outline: 1px solid #9fcfff!important; } | ||
| &.is-focused{ |
| &:focus{ outline: 1px solid #9fcfff!important; } | ||
| &.is-focused{ | ||
| outline: none; | ||
| .choices__inner{ border-radius: 0.25rem 0.25rem 0 0; } |
| @@ -99,12 +99,96 @@ | |||
| } | |||
|
|
|||
| .citizen-filter { | |||
| &>div{ | |||
There was a problem hiding this comment.
This css is for spacing on mobile I guess
Can we change it by adding mb-2 class to each column inside .citizen-filter?
| let selectTinh = new Choices(document.getElementById('select-tinh')); | ||
| let selectHuyen = new Choices(document.getElementById('select-huyen')); | ||
| let selectXa = new Choices(document.getElementById('select-xa')); | ||
| let selectStatus = new Choices(document.getElementById('select-status')); |
There was a problem hiding this comment.
In the issue, there is no requirement to add a search to select status and just a few items, we don't need search for status.
|
|
||
| // Then update the choices: | ||
| selectHuyen.clearChoices() | ||
| selectHuyen.setChoices(listHuyen) | ||
| }); |
There was a problem hiding this comment.
I believe there is a Tab here, can you change it to space?
| border:none; | ||
| border-bottom: 1px solid #ced4da; | ||
| padding: 0.25rem 0.4rem; | ||
| &:focus{ |
|
@dshongphuc anh mới hỏi bên cái issue related đến cái PR này: #209 |
|
Hi anh @mihnsen,
Tuy nhiên, chỉ có cái vấn đề với Package-lock.json là em chưa rõ lắm, lúc em cài package mới hoặc install các packages hiện có trong package.json, nó luôn báo : Thanks anh nhiều ! |
|
@dshongphuc anh thấy các bạn định thay đổi model hộ dân. em xem thêm chi tiết ở đây nhé: #208 |
|
hi anh @mihnsen , em đã thảo luận với bạn @kc97ble mấy hôm trước và bạn ấy đã confirm là issue này ko liên quan đến #208 ạ : #209 (comment) |
|
@mihnsen anh có thể review lại giúp em cái PR này không ạ? Nếu ổn thì ta có thể merge. |
Mình thêm tính năng Cho phép gõ tên trong phần lọc tỉnh/huyện/xã #209 trong page Hộ dân.
Mình test khá kĩ rồi nhưng nếu có vấn đề gì vui lòng cho mình biết để mình fix nhé reviewers. :)