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

editor: fix missing stars for required fields #163

Merged
merged 1 commit into from
Apr 6, 2020

Conversation

jma
Copy link
Contributor

@jma jma commented Apr 3, 2020

  • Fixes missing stars after required field titles when the editor is displayed.
  • Fixes the number of items after the hiding and showing array fields.
    It should be equal to the minimum number of items defined in the JSONSchema.
  • Fixes missing title for array of array fields.

Co-Authored-by: Johnny Mariéthoz johnny.mariethoz@rero.ch
Co-Authored-by: Gianni Pante gianni.pante@rero.ch

Why are you opening this PR?

How to test?

  • Deploy with rero-ils: data model: implement bnf title and variants transformation rero-ils#864

  • Check that a start is displayed next to the title.
    image

  • Check that the Publication statement is displayed correctly (array of array)
    image

  • Take an array element add several values, hide the field, show the field. The field should be as the initial state of the editor (creation).

Code review check list

  • Commit message template compliance.
  • Commit message without typos.
  • File names.
  • Functions names.
  • Functions docstrings.
  • Unnecessary commited files?
  • Extracted translations?

@jma jma force-pushed the maj-required branch 3 times, most recently from 5452085 to c4b3190 Compare April 3, 2020 16:42
@jma jma marked this pull request as ready for review April 3, 2020 16:48
@jma jma requested review from sebdeleze and AoNoOokami April 3, 2020 16:49
Copy link

@sebdeleze sebdeleze left a comment

Choose a reason for hiding this comment

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

  • There's a typo in message commit: after required field.
  • And here, seems to be: items after the hide and show (without "a").
  • Maybe in the title, you should use "required" instead of "require" be I'm not sure..

&& (this.field.id === changes.field.id)
) {
// number of elements to remove
const nToRemove = this.field.fieldGroup.length - minItems;

Choose a reason for hiding this comment

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

Better to use explicit names for constants and variables.

* Fixes missing stars after required field titles when the editor is displayed.
* Fixes the number of items after the hiding and showing array fields.
It should be equal to the minimum number of items defined in the JSONSchema.
* Fixes missing title for array of array fields.

Co-Authored-by: Johnny Mariéthoz <johnny.mariethoz@rero.ch>
Co-Authored-by: Gianni Pante <gianni.pante@rero.ch>
@jma jma changed the title editor: fix missing stars for require fields editor: fix missing stars for required fields Apr 4, 2020
@sebdeleze sebdeleze merged commit 8416443 into rero:dev Apr 6, 2020
@jma jma deleted the maj-required branch April 10, 2020 07:53
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