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

[PR] People record editing #64

Merged
merged 6 commits into from
Apr 19, 2017
Merged

[PR] People record editing #64

merged 6 commits into from
Apr 19, 2017

Conversation

philcable
Copy link

More work on #55. I think this gets us nearly there - a better way to handle the permissions checking and an indication that changes will be pushed back to people.wsu.edu might be all we need to close the issue.

Phil Cable added 6 commits April 12, 2017 15:13
This is super basic - just want to get it commited before making
more substantial changes.
This might be madness, but it seemed like the right course
yesterday afternoon, so I'm going to stick to it.

The code in `admin-edit-profile-secondary.js` is only needed on
sites other than the primary directory - even though some of its
functions are referenced in `admin-edit-profile.js`. But, those
functions will only be called on sites other than the primary
directory.

I've occasionally seen an error where the script is trying to
`setContent` for the editors before TinyMCE is initialized, which
informed the decision to enqueue `admin-edit-profile-secondary.js`
from within `wp_enqueue_editor`. I haven't seen it since, so I'm
hoping this has made it more reliable.

The `secondary` script checks for changes before posting back to
people.wsu.edu. This is probably overkill, but it seemed like a
good idea at the time.

Lastly, this is the start of an effort to (re)name JavaScript
files in a way that better indicates where they're used.
I've been horribly inconsistent with my syntax in the JavaScript
for this plugin, so this is an attempt to start rectifying that.
This is pretty bad, but it gets the general idea across. Even if a
user doesn't have sufficient permissions to edit a profile, they
should still be able to select which Working Title, Biography, and
Photo to display on the front end for their particular site. So,
this demonstrates how that could work - though I'm sure there's a
nicer way to do it. Like one that doesn't throw a console error.

I'm also wondering if doing it the other way around would make
more sense (that is, disable all fields by default and only enable
them if a user has permissions to edit the profile).
@philcable
Copy link
Author

@jeremyfelt #reviewmerge?

Copy link
Contributor

@jeremyfelt jeremyfelt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@jeremyfelt jeremyfelt merged commit cf0444e into master Apr 19, 2017
@jeremyfelt jeremyfelt deleted the profile-info-sync branch April 19, 2017 18:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants