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: shows/hides main (1th level) fields #473

Merged
merged 1 commit into from
Sep 16, 2019

Conversation

jma
Copy link
Contributor

@jma jma commented Sep 3, 2019

  • Add buttons to editor to hide or show optional fields.
  • With initial data, populated fields are diplayed, empty fields are
    hidden.
  • All document fields are available in the editor.
  • Fixe language and identifiedBy fields for BNF importation.

Co-Authored-by: Johnny Mariéthoz Johnny.Mariethoz@rero.ch

@jma jma force-pushed the maj-#962-add-dynamic-fields branch 2 times, most recently from c9c9bc0 to 50d69db Compare September 3, 2019 11:39
@jma jma added WIP and removed WIP labels Sep 3, 2019
@jma jma force-pushed the maj-#962-add-dynamic-fields branch from 50d69db to 2646208 Compare September 3, 2019 13:22
@jma jma force-pushed the US911-cataloging branch from 5277bf9 to 9d902df Compare September 3, 2019 14:02
@jma jma force-pushed the US911-cataloging branch from 9d902df to 5d017de Compare September 3, 2019 14:38
@jma jma force-pushed the maj-#962-add-dynamic-fields branch from 2646208 to daaa18e Compare September 3, 2019 14:38
@jma jma force-pushed the maj-#962-add-dynamic-fields branch from daaa18e to 8102c52 Compare September 3, 2019 15:16
@jma jma requested a review from pronguen September 4, 2019 05:06
Copy link

@BadrAly BadrAly left a comment

Choose a reason for hiding this comment

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

Fix typos in commit message.

Copy link
Contributor

@pronguen pronguen left a comment

Choose a reason for hiding this comment

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

Problems:

  • In 2nd-level fields, selecting text with the mouse is not possible. However, it is possible in the 1st-level fields.
  • Repetitive subfields (publisher.name, publisher.place) are not fully displayed in the editor: only one is displayed.
  • Existing ISBN are displayed in the detailed view, but if I edit a record, it disappears from the detailed view (same if I create a new record).
  • When adding a second "language", the key "type" (hidden in the form) is not added and the form cannot be validated and saved.

Improvements:

  • When adding a subfield with the "plus" or when adding a field listed in the top section, place the mouse cursor directly in the new input box.
  • When adding a field listed in the top section, display the form so that the newly added field is visible.
  • In the top section (add fields), list all kind of fields that can be added, even those already in the form that are repetitive.
  • Some constraints should be added for the field "identifiedBy":
    • "source" exists only if type=bf:Local and is in this case required
    • "acquisitionsTerms" exists only if type=bf:Isbn

Copy link

@iGormilhit iGormilhit left a comment

Choose a reason for hiding this comment

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

Some minor changes in the commit message:

editor: shows/hides main (1st level) fields

* Adds buttons to editor to hide or show optional fields.
* With initial data, populated fields are displayed, empty fields are
hidden.
* All document fields are available in the editor.
* Fixes language and identifiedBy fields for BNF importation.

@jma jma force-pushed the maj-#962-add-dynamic-fields branch 2 times, most recently from c2c0835 to 155a3f3 Compare September 5, 2019 14:54
* Adds buttons to editor to hide or show optional fields.
* With initial data, populated fields are displayed, empty fields are
hidden.
* All document fields are available in the editor.
* Fixes language and identifiedBy fields for BNF importation.

Co-Authored-by: Johnny Mariéthoz <Johnny.Mariethoz@rero.ch>
@jma jma force-pushed the maj-#962-add-dynamic-fields branch from 155a3f3 to 168cd9a Compare September 5, 2019 14:55
@jma
Copy link
Contributor Author

jma commented Sep 5, 2019

Some minor changes in the commit message:

editor: shows/hides main (1st level) fields

* Adds buttons to editor to hide or show optional fields.
* With initial data, populated fields are displayed, empty fields are
hidden.
* All document fields are available in the editor.
* Fixes language and identifiedBy fields for BNF importation.

Done

@jma
Copy link
Contributor Author

jma commented Sep 5, 2019

Problems:

  • In 2nd-level fields, selecting text with the mouse is not possible. However, it is possible in the 1st-level fields.

Not fixed, you can open an issue if you want.

  • Repetitive subfields (publisher.name, publisher.place) are not fully displayed in the editor: only one is displayed.

This is an issue of the editor library. An issue has been opened: hamzahamidi/ajsf#144

  • Existing ISBN are displayed in the detailed view, but if I edit a record, it disappears from the detailed view (same if I create a new record).

Fixed.

  • When adding a second "language", the key "type" (hidden in the form) is not added and the form cannot be validated and saved.

Fixed.

Improvements:

  • When adding a subfield with the "plus" or when adding a field listed in the top section, place the mouse cursor directly in the new input box.
    To be done.
  • When adding a field listed in the top section, display the form so that the newly added field is visible.
    To be done.
  • In the top section (add fields), list all kind of fields that can be added, even those already in the form that are repetitive.
    To be done.
  • Some constraints should be added for the field "identifiedBy":

    • "source" exists only if type=bf:Local and is in this case required

Done but required part (not possible with the actual version of jsonschema as I know).

  • "acquisitionsTerms" exists only if type=bf:Isbn
    Done.

@jma jma requested review from iGormilhit and BadrAly and removed request for iGormilhit September 5, 2019 15:10
Copy link
Contributor

@rerowep rerowep left a comment

Choose a reason for hiding this comment

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

Rebase with dev please for faster deployment!

@jma jma merged commit fc8d772 into rero:US911-cataloging Sep 16, 2019
@jma jma deleted the maj-#962-add-dynamic-fields branch December 11, 2019 09:28
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.

5 participants