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.

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

Issue #33 : Participant bio popover for "other" participants #36

Merged

Conversation

bcholmes
Copy link
Collaborator

Make the following changes:

  • fetch the participant info via a Rest/JSON endpoint, rather than an XML interface
  • tweak the code that manages the popover (instances of this.this.X were making my eyes bleed)
  • improve some basic name handling (create a PersonName from a database record; render a PersonName as an array, ready to be rendered to JSON).

…se JSON, rather than XML) and tweak the handling of the popover that would display this
@bcholmes bcholmes linked an issue Mar 27, 2022 that may be closed by this pull request
@LVerhulst4321
Copy link
Owner

Thank you! I'll make sure to examine this and hopefully learn from it so I can do this myself one day.

@LVerhulst4321 LVerhulst4321 merged commit eb3e740 into LVerhulst4321:main Mar 28, 2022
@bcholmes bcholmes deleted the assign-participant-popover-bug branch March 28, 2022 19:18
@lostcarpark
Copy link
Collaborator

The new code looks very tidy.
However, I have a slight concern that there's a lot of code added, but very little taken away. Does the JS for the old method still need to be on the form? Could the old code be stripped out of SubmitAdminParticipants.php?
I don't like leaving dead code in a project.

@lostcarpark
Copy link
Collaborator

One other small point. The bio pops up on the left edge of the participant drop-down. This puts it at the left edge of the page, which I find can be a little hard to read. Just wondering what you'd think of attaching the pop-up to the "show bio" button instead of the drop-down?

@LVerhulst4321
Copy link
Owner

James, the old code for fetch_participant is still being referenced by a few other spots in the code, so it cannot be stripped out at this time. Once the other spots are updated (which isn't a priority), then the old code can be removed.

Moving the pop-up is fine. It is a little hard to read over there.

LVerhulst4321 added a commit that referenced this pull request Mar 29, 2022
Fix the fetch participant bug reported in issue #36
bcholmes added a commit to bcholmes/PlanZ that referenced this pull request Mar 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Issue with Show Bio button at bottom of Assign Participant screen
3 participants