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

Form disable meta #133

Merged
merged 7 commits into from
Jan 15, 2019
Merged

Form disable meta #133

merged 7 commits into from
Jan 15, 2019

Conversation

richardhallett
Copy link
Contributor

For #132

@richardhallett richardhallett self-assigned this Jan 14, 2019
@ghost ghost added the development label Jan 14, 2019
this._super(...arguments);

if (this.model.get("source") == "fabricaForm") {
this.set('metadataFieldsDisabled', false);
Copy link
Contributor

@mfenner mfenner Jan 14, 2019

Choose a reason for hiding this comment

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

You could set a default in line 4.

export default Component.extend({
  metadataFieldsDisabled: true,

  didReceiveAttrs() {
    if (this.model.get("source") == "fabricaForm") {
      this.set('metadataFieldsDisabled', false);
    }

Copy link
Contributor

@mfenner mfenner left a comment

Choose a reason for hiding this comment

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

Why is there a doi-form component. It seems to add complexity.

@richardhallett
Copy link
Contributor Author

Why is there a doi-form component. It seems to add complexity.

Because we need specific functionality that applies to the form, this seemed a logical way to encapsulate the behaviour of the metadata form fields separate to that of other parts.
Do you have another suggestion of where to put the logic around deciding if the form elements should be disabled or not?

@mfenner
Copy link
Contributor

mfenner commented Jan 15, 2019

@richardhallett the form is displayed in four slightly different versions: new/edit and form/file upload. We need to disable the form in only one of them (edit form). So it would make sense for me to put that logic into the template for edit form: https://github.com/datacite/bracco/blob/master/app/templates/clients/show/dois/show/edit.hbs

@richardhallett
Copy link
Contributor Author

@richardhallett the form is displayed in four slightly different versions: new/edit and form/file upload. We need to disable the form in only one of them (edit form). So it would make sense for me to put that logic into the template for edit form: https://github.com/datacite/bracco/blob/master/app/templates/clients/show/dois/show/edit.hbs

The usage of the field could be in that I suppose, but given I needed to set the metadataFieldsDisabled this seemed best to put it in component code which is where the backing of doi-form.js comes in? Where instead would you put that in place, create a backend component logic for the show/edit.js?

@richardhallett richardhallett merged commit 48de096 into master Jan 15, 2019
@ghost ghost removed the development label Jan 15, 2019
@richardhallett richardhallett deleted the form-disable-meta branch January 15, 2019 12:53
@@ -23,6 +23,8 @@ export default Controller.extend({
doi.set('descriptions', [{ description: doi.get('descriptions'), descriptionType: 'Abstract' }]);
}

doi.set("source", "fabricaForm");
Copy link
Contributor

Choose a reason for hiding this comment

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

@richardhallett you can also do this directly when you create the DOI, i.e. start with defaults.

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.

2 participants