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

Use molecule filename as index #110

Merged
merged 10 commits into from
Aug 10, 2022
Merged

Use molecule filename as index #110

merged 10 commits into from
Aug 10, 2022

Conversation

sverhoeven
Copy link
Member

@sverhoeven sverhoeven commented Jul 29, 2022

Fixes #57
Fixes #76

TODO

  • lookup array as ui:indexable value
  • after molecule files have changed the uiSchemas with the ui:indexable parameter with maxItemsFrom > moleculeinputpaths in schema should be updated

Previous PR #104 was merged prematurly, this PR continues work.

To test:

  1. In haddock3-download app
    1. upload pdb file into global parameters
    2. Add mdref node
    3. In form of mdref expand molecules subform
    4. Press + button on mol_shape parameter
    5. The filename of the uploaded pdb file should show as first column
  2. In storybook of form package yarn storybook + http://localhost:6007/
    1. The Array and TableField have stories with indexes.

Due to formatting with prettier and yarn lint --fix there are changes that are not relevant.

@sverhoeven sverhoeven marked this pull request as draft July 29, 2022 11:12
@netlify
Copy link

netlify bot commented Jul 29, 2022

Deploy Preview for wonderful-noether-53a9e8 ready!

Name Link
🔨 Latest commit 918f5e6
🔍 Latest deploy log https://app.netlify.com/sites/wonderful-noether-53a9e8/deploys/62f366e3d4cf250009b202fd
😎 Deploy Preview https://deploy-preview-110--wonderful-noether-53a9e8.netlify.app/
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@codecov-commenter
Copy link

codecov-commenter commented Aug 5, 2022

Codecov Report

Merging #110 (c73a9da) into main (32cc0da) will increase coverage by 2.77%.
The diff coverage is 86.30%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #110      +/-   ##
==========================================
+ Coverage   61.29%   64.06%   +2.77%     
==========================================
  Files          55       56       +1     
  Lines        3741     3974     +233     
  Branches      275      317      +42     
==========================================
+ Hits         2293     2546     +253     
+ Misses       1444     1426      -18     
+ Partials        4        2       -2     
Flag Coverage Δ
core-unit 60.36% <86.60%> (+3.57%) ⬆️
form-unit 78.47% <84.09%> (+0.66%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
packages/core/src/store.ts 75.88% <43.47%> (+3.25%) ⬆️
packages/form/src/ArrayFieldTemplate.tsx 74.41% <63.15%> (+0.47%) ⬆️
...ackages/core/src/molecule/addMoleculeValidation.ts 92.81% <93.28%> (+1.14%) ⬆️
packages/core/src/NodeForm.tsx 44.44% <100.00%> (+44.44%) ⬆️
packages/core/src/molecule/parse.ts 92.10% <100.00%> (+2.81%) ⬆️
packages/core/src/validate.ts 97.77% <100.00%> (+0.05%) ⬆️
packages/form/src/table/TableFieldTemplate.tsx 71.69% <100.00%> (-0.36%) ⬇️
packages/form/src/useIndexable.tsx 100.00% <100.00%> (ø)

…on molecule path

Refactored code to do parseMolecules only once.

Ran pretier on packages/core/src/molecule/addMoleculeValidation.test.ts
@sverhoeven sverhoeven marked this pull request as ready for review August 5, 2022 11:08
@sverhoeven sverhoeven requested a review from bvreede August 5, 2022 11:23
Copy link
Contributor

@bvreede bvreede left a comment

Choose a reason for hiding this comment

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

Works, very nice!

The only thing I would change in this PR is the location of the molecule ID in the topoaa form. I would either top-outline the left hand column, so that the name of the molecule is at the start of the form, or forego columns altogether and use the ID in a title field.

The reason is that with a large form on the right hand side, as is the case with topoaa, the name of a molecule will show up about half way; in addition, there is no clearly visible separation between the two molecules, which means that the user has to pay close attention when filling out the form which question belongs to which molecule.

E.g. in the example below, "hise" belongs to the first molecule, "segment ID" to the second.

Screenshot 2022-08-09 at 15 04 17

@sverhoeven
Copy link
Member Author

Thanks for reviewing.

Good suggestion about position, I moved the index label to the top when the children are an object or array.

@sverhoeven sverhoeven merged commit e039114 into main Aug 10, 2022
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.

Give index to uploaded molecules to make clear which paramters belong to which molecule Array index in form
3 participants