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

Feature/extend arguments widget #404

Closed
wants to merge 11 commits into from

Conversation

LorenzoJokhan
Copy link
Contributor

Description

Please include

  • a summary of the changes
  • relevant motivation and context
  • a list of any dependencies that are required for this change

Issue reference

Fixes # (issue)

Type of change

Is it a new feature, bug fix, code improvement, etc.
If it is a breaking change what needs to be done to fix that

Documentation

Is the documentation updated, maybe a link

Tests

(How) has the change been tested

Branch

If the branch to merge to is not development

Copy link
Contributor

Choose a reason for hiding this comment

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

The field is still commented so it won't show.

@@ -86,28 +81,60 @@ module.exports = {
{
name: 'advanced',
label: 'Advanced',
fields: ['replyingEnabled', 'votingEnabled']
fields: ['replyingEnabled', 'votingEnabled', 'showLastNameForArguments', 'showLastNameForReactions', 'ideaId']
Copy link
Contributor

Choose a reason for hiding this comment

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

showLastNameForArguments and showLastNameForReactions are not defined in this file. I don't think they belong in this PR because they are not within the scope of the requested change — we only need the ideaId in this field.

Comment on lines +128 to +138
self.addHelpers({
showLastName: function (type, widget, user) {
if (type == 'reactions' && (widget.showLastNameForReactions == 'yes' || (widget.showLastNameForReactions == 'adminonly' && user.role == 'admin'))) {
return user.lastName;
} else if (type == 'arguments' && (widget.showLastNameForArguments == 'yes' || (widget.showLastNameForArguments == 'adminonly' && user.role == 'admin'))) {
return user.lastName;
}

return '';
}
});
Copy link
Contributor

Choose a reason for hiding this comment

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

Same goes for this helper — should not be in this PR I think.

@LorenzoJokhan
Copy link
Contributor Author

#416 Vervangt deze PR

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.

3 participants