search

archesproject / arches-her

5 stars 12 forks source link

#1196 Upgrade and add Generate Unique References function #1206

Closed csmith-he closed 1 month ago

csmith-he commented 1 month ago

Generate unique references function added to project and package, and updated to handle new internationalisation format for strings.

Fulfils #1196

csmith-he commented 1 month ago

@SDScandrettKint would you be ok reviewing this PR?

SDScandrettKint commented 1 month ago

During this review I noticed that the dropdown options are cut off towards the bottom @phudson-he

This seems to happen more severely when the nodegroup has a larger number of nodes.

1) image

2) https://github.com/archesproject/arches-her/assets/119508494/4592e445-4b7a-48c9-b0de-b397e5a7d014

3) https://github.com/archesproject/arches-her/assets/119508494/b50967d2-e9d4-4bae-9447-da76c408d700

Further, there is a descrepency as to whether the dropdown has a search bar or not (comparing 2 and 3). I can't determine if this is related to the number of nodes in a nodegroup (e.g. a nodegroup with <10 nodes doesn't have a search bar), however, I don't think it is the fault of this PR and instead a more general Arches issue.

EDIT: It seems that there are two y axis scroll bars that influence this, example below:

https://github.com/archesproject/arches-her/assets/119508494/9c1b8d46-8472-4b8a-be70-8a95d9c7a791

csmith-he commented 1 month ago

@SDScandrettKint

I've added the fix you identified to stop the doubling of the triggering_nodegroups array - thanks for finding that and suggesting a solution.

I've also put a hotfix in the project.css to sort out the dropdown list being cut off so it works as expected for now. I'll add an issue to the Roadmap Backlog for the dropdown to use SelectWoo rather than Chosen; doing that work now will probably hold us up with getting v1 of Arches for HERs out so we'll come back to that.

@aj-he and I had a chat about narrowing down the options in the dropdown lists. For the moment, we'd like to keep things as is as it will allow people to use the function in other configurations if they end up making their own models - if it causes widespread problems we can always change this at a later date.

Would you mind reviewing again with the changes in, please?