gramps-web
gramps-web copied to clipboard
gramps-project
Improve names edit handling (fixes #123) (fixes #149)
Added some fixes and improvements in names' edit handling.
Fixes:
- name-type not selected
- fix show-more feature
Features:
- add sort to surname
- add delete to name and surname
UX/UI improvements:
- use editable list for names
- add name edit form validation
- improve name-type vs. custom-type handling
- some minor styling
Fixes:
- https://github.com/gramps-project/gramps-web/issues/149
- https://github.com/gramps-project/gramps-web/issues/123
Great, thanks! Let me know when it's ready for review.
One comment: please don't modify the langugage JSONs, they are updated via Weblate. Unfortunately changing the english string is only possible by adding a new one (unless I'm mistaken).
Great, thanks! Let me know when it's ready for review.
One comment: please don't modify the langugage JSONs, they are updated via Weblate. Unfortunately changing the english string is only possible by adding a new one (unless I'm mistaken).
I was also unsure about the language jsons. I will change it.
Generally I am not a JavaScript crack at all, so I am really happy for feedback for improvements. 😄 Also I have still issues getting the test locally to run. I guess after fixing this, I might be review ready. I let you know when I am ready.
Also I have still issues getting the test locally to run. I guess after fixing this, I might be review ready. I let you know when I am ready.
Unfortunately there are no unit tests for this repo as I'm not good with Javascript unit tests :slightly_frowning_face: . Are you referring to the Web API (Python) tests?
I couldn't resist to already add a couple of comments. The code looks really good overall!
Also I have still issues getting the test locally to run. I guess after fixing this, I might be review ready. I let you know when I am ready.
Unfortunately there are no unit tests for this repo as I'm not good with Javascript unit tests 🙁 . Are you referring to the Web API (Python) tests?
I haven't really gotten into this by now. I just thought there were some, because there is this "test"-script in the package.json. But okay, then I will ignore this. Maybe I can add some someday 😄
Great, thanks! Let me know when it's ready for review.
One comment: please don't modify the langugage JSONs, they are updated via Weblate. Unfortunately changing the english string is only possible by adding a new one (unless I'm mistaken).
I am still not quite sure how to introduce new strings. I found the strings.js, but those translations seem to come from the backend. I saw in one commit a string added to the en.json? So this seems to be the way to go!?!
Or does Weblate identifies them inside the code? (I doubt it. 😄 )
I tried out your code, great work! It brings the whole name editing experience to a new level. Just two small things:
- The only feature that's still missing is changing the primary name. That would be relatively simple to implement I guess using e.g. a "star" button. Up to you if you want to add it, we can always add it later
- I was confused by the presence of the delete and up/down buttons if the name details are not expanded yet, because they have no effect/meaning:
Can you hide them?
I tried out your code, great work! It brings the whole name editing experience to a new level. Just two small things:
* The only feature that's still missing is changing the primary name. That would be relatively simple to implement I guess using e.g. a "star" button. Up to you if you want to add it, we can always add it later * I was confused by the presence of the delete and up/down buttons if the name details are not expanded yet, because they have no effect/meaning:Can you hide them?
Thanks a lot! And yes!
1.I wanted to add changing the primary name. Thanks for the hint. Just forgot it last week. 😄 2. You are right. I will hide the editing options until the details are shown. When you have more names in the list, it's not so bad. But I definitely see the point.
Hi @BlitzundBogen should we merge it as is right now or two you want to implement the two points above? However there is a conflict right now.
Hi @BlitzundBogen should we merge it as is right now or two you want to implement the two points above? However there is a conflict right now.
I am sorry, I didn't have the time to improve. I will try to at least solve the conflicts this week. If I do not find the time I will definitely find it the week after. Then I could also do the improvements.
I had some issues with my dev environment again, which took me some time to solve. For now only the order of names/changing primary name is missing. I would suggest, if you are ready to merge, just do it and I will open a new PR for this issue. If I am faster, i will add it here. But currently I doubt that. 😄
PS: David, what happened to you profile? I cannot find you anymore. 😱
I am a big fan of these changes, thank you for taking care. Looking forward to do thorough testing and giving feedback. (I have no dev setup yet, so I will have to wait until the release. Is there documentation for a proper dev setup?)
Is there documentation for a proper dev setup
Not sure about the "proper" :wink: but here is the documentation: https://www.grampsweb.org/dev-frontend/setup/
PS: David, what happened to you profile? I cannot find you anymore. 😱
Yes, Github (or its AI) suspended my account (in error) and it took them one month to process my appeal :sob: