Improve names edit handling (fixes #123) (fixes #149) #404

BlitzundBogen · 2024-03-15T04:35:36Z

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:

DavidMStraub · 2024-03-15T07:51:59Z

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).

BlitzundBogen · 2024-03-15T15:46:42Z

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.

DavidMStraub · 2024-03-15T16:17:38Z

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?

src/components/GrampsjsPerson.js

DavidMStraub · 2024-03-16T08:30:53Z

src/components/GrampsjsPerson.js

        : given
-    return html`${given} ${surname} ${suffix}`
+    return html`${given} ${nick ? `"${nick}" ` : ''}${surname} ${suffix}`


Would be nice to use typographical quotation marks (although strictly speaking that would even by locale dependent)

But on second thought I am not sure I would like to always have the nickname displayed as part of the page title ... does Gramps Desktop do it like that? I think we can use that as guidance to have a more consistent user experience. For instance, if both Gramps & Gramps Web would display nicknames as part of the person profile's title, I would likely want to move more informal nick names to a secondary name to prevent them from showing up...

How do you deal with those kind of suggestions in the project? Should I open a discussion here in gitlab?

But you are right. I guess the best thing would be, if the user could choose (and define it in the user settings).

The scenario I came up with this was: It used to be like that, that parents and children often share the same given name. Sometimes they differ in second or third given names (which might not be known). To make it easier to distinguish persons, this might help.

I will remove this for now.

In my family tree there are some persons with this issue 😄 This is why they mostly have a nickname. To distinguish e.g. between mother and daughter.

Maybe I can implement this when solving #67

DavidMStraub · 2024-03-16T08:34:38Z

src/components/GrampsjsFormSelectType.js

+                for="button-switch-type"
+                content="${this._hasCustomType
+                  ? this._('Switch to default type')
+                  : this._('Add custom type')}"


I suggest to leave the string as it was orginally to not lose the translations.

I can do that. Still I would like to propose to add the string 'Add custom type' for this scenario. It's more self explaning. But I woulf leave it like it was for now. 😄

Or maybe I add the string. From UX-perspective I see it like this:
If I set a custom type and save it will become a "default type" (as it is now in the default types list in the frontend). When users read "switch" they might expect to see the custom type set before there.

Not sure what you mean. There are two kinds of types in Gramps: default types and custom types. The default types are translated, while the custom types are just arbitrary strings. You can either select from a list of default types, or use a custom type instead. If you want to replace a custom type by a default type again, you have to switch back to the dropdown list.

After trying it out I get your point and I think you're right 😄 I missed the fact that the dropdown contains both custom and default types.

So: great idea.

DavidMStraub · 2024-03-16T08:35:05Z

I couldn't resist to already add a couple of comments. The code looks really good overall!

BlitzundBogen · 2024-03-21T17:34:10Z

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 😄

BlitzundBogen · 2024-03-21T18:20:55Z

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. 😄 )

DavidMStraub · 2024-03-25T20:21:05Z

Yes, just add new strings to en.json.

DavidMStraub · 2024-03-25T20:35:45Z

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?

BlitzundBogen · 2024-03-27T19:07:13Z

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.

DavidMStraub · 2024-04-15T16:22:42Z

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.

BlitzundBogen · 2024-04-21T19:15:02Z

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.

BlitzundBogen · 2024-05-14T10:44:58Z

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. 😱

m-breitbach · 2024-06-14T06:09:41Z

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?)

DavidMStraub · 2024-06-14T06:18:52Z

Is there documentation for a proper dev setup

Not sure about the "proper" 😉 but here is the documentation: https://www.grampsweb.org/dev-frontend/setup/

DavidMStraub · 2024-06-29T09:41:09Z

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 😭

DavidMStraub · 2024-06-29T09:44:32Z

I'm rebasing now.

DavidMStraub · 2024-06-29T11:02:26Z

🎉

…project#149) (gramps-project#404) * Fix type not shown as selected * Align names edit ux as editable list * Add delete name * Add validation to name form * Improve name type ux * Use classMap instead of style * Add ui improvements to surnames * Fix show more in name edit * Add edit features to surnames * Add nick to name * Use existing strings * Adjust call and remove nickname * Add language strings * Only show edit buttons on "show more" --------- Co-authored-by: BlitzundBogen <code@blitzundbogen.de>

BlitzundBogen changed the title ~~Improve names handling (fixes #123) (fixes #149)~~ Improve names edit handling (fixes #123) (fixes #149) Mar 15, 2024

DavidMStraub reviewed Mar 16, 2024

View reviewed changes

src/components/GrampsjsPerson.js Outdated Show resolved Hide resolved

DavidMStraub reviewed Mar 16, 2024

View reviewed changes

BlitzundBogen marked this pull request as ready for review March 21, 2024 18:50

BlitzundBogen added 10 commits June 29, 2024 11:42

Fix type not shown as selected

e196ff1

Align names edit ux as editable list

e5c61dc

Add delete name

bbe5353

Add validation to name form

4cafa81

Improve name type ux

268ef43

Use classMap instead of style

9f27027

Add ui improvements to surnames

63650a4

Fix show more in name edit

9eb23b4

Add edit features to surnames

63b066d

Add nick to name

183eb21

BlitzundBogen added 4 commits June 29, 2024 11:42

Use existing strings

eadf738

Adjust call and remove nickname

dcbe538

Add language strings

718064d

Only show edit buttons on "show more"

b589112

DavidMStraub force-pushed the feature/improve-names-handling branch from 6999dd9 to b589112 Compare June 29, 2024 10:43

DavidMStraub merged commit fdebaf5 into gramps-project:main Jun 29, 2024
1 check passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve names edit handling (fixes #123) (fixes #149) #404

Improve names edit handling (fixes #123) (fixes #149) #404

BlitzundBogen commented Mar 15, 2024 •

edited

Loading

DavidMStraub commented Mar 15, 2024

BlitzundBogen commented Mar 15, 2024

DavidMStraub commented Mar 15, 2024

DavidMStraub Mar 16, 2024

DavidMStraub Mar 16, 2024

BlitzundBogen Mar 21, 2024

BlitzundBogen Mar 21, 2024

DavidMStraub Mar 16, 2024

BlitzundBogen Mar 21, 2024

BlitzundBogen Mar 21, 2024 •

edited

Loading

DavidMStraub Mar 25, 2024

DavidMStraub Mar 25, 2024

DavidMStraub commented Mar 16, 2024

BlitzundBogen commented Mar 21, 2024

BlitzundBogen commented Mar 21, 2024

DavidMStraub commented Mar 25, 2024

DavidMStraub commented Mar 25, 2024

BlitzundBogen commented Mar 27, 2024 •

edited

Loading

DavidMStraub commented Apr 15, 2024

BlitzundBogen commented Apr 21, 2024

BlitzundBogen commented May 14, 2024

m-breitbach commented Jun 14, 2024

DavidMStraub commented Jun 14, 2024

DavidMStraub commented Jun 29, 2024

DavidMStraub commented Jun 29, 2024

DavidMStraub commented Jun 29, 2024

Improve names edit handling (fixes #123) (fixes #149) #404

Improve names edit handling (fixes #123) (fixes #149) #404

Conversation

BlitzundBogen commented Mar 15, 2024 • edited Loading

DavidMStraub commented Mar 15, 2024

BlitzundBogen commented Mar 15, 2024

DavidMStraub commented Mar 15, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BlitzundBogen Mar 21, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DavidMStraub commented Mar 16, 2024

BlitzundBogen commented Mar 21, 2024

BlitzundBogen commented Mar 21, 2024

DavidMStraub commented Mar 25, 2024

DavidMStraub commented Mar 25, 2024

BlitzundBogen commented Mar 27, 2024 • edited Loading

DavidMStraub commented Apr 15, 2024

BlitzundBogen commented Apr 21, 2024

BlitzundBogen commented May 14, 2024

m-breitbach commented Jun 14, 2024

DavidMStraub commented Jun 14, 2024

DavidMStraub commented Jun 29, 2024

DavidMStraub commented Jun 29, 2024

DavidMStraub commented Jun 29, 2024

BlitzundBogen commented Mar 15, 2024 •

edited

Loading

BlitzundBogen Mar 21, 2024 •

edited

Loading

BlitzundBogen commented Mar 27, 2024 •

edited

Loading