Page MenuHomeSoftware Heritage

added some fields from the SWHAPPE example; added some TODOs
ClosedPublic

Authored by scatenag on Nov 5 2019, 10:19 AM.

Diff Detail

Repository
rDWCM CodeMeta Generator (mirror)
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

Hi Guido,

You need to sign L3 in order for us to read the content of your diff.

Thanks!

Thanks @scatenag for this !
follow the instructions of @vlorentz about L3 and we can continue the work on this tool :-)

codemeta_generation.js
44

you can add operatingSystem if you fix line 86 :-)

index.html
37

I looked at the schema.org definition of identifier (https://schema.org/identifier),
it can be text, url or PropertyValue.
Used as follows in data representation with microdata:

<div property="identifier" typeof="PropertyValue">
        <span property="propertyID">DOI</span>: 
         <span property="value">10.151.xxxxx</span>
 </div>

we can use that with identifier-type and identifier-value to have a clearer idea of what needs to be in the input.

85

Seems there are two properties here,operatingSystem and runtimePlatform

The description of runtimePlatform is more about dependencies:

Runtime platform or script interpreter dependencies (Example - Java v1, Python2.3, .Net Framework 3.0). Supersedes runtime.

101

the text should be isPartOf instead of funding

vlorentz requested changes to this revision.Nov 5 2019, 2:21 PM

follow the instructions of @vlorentz about L3 and we can continue the work on this tool :-)

This is already done, else you wouldn't be able to read the code :)

dynamic_form.js
38–41

nitpick: indentation of the <p> block

index.html
74

The id of this fieldset should be changed to fieldsetAdditionalInfo. (And possibly, move the fieldset at the end, as it's for fields that don't fit other ones)

79

Description should be in the text, not the placeholder. Placeholders are for examples.

85

Same, placeholder should be an example

91

You could use a datalist for this, so it auto-completes when users write it. Example: https://developer.mozilla.org/fr/docs/Web/HTML/Element/datalist

Possibly with a validator pattern (as for dateCreated) if we want to validate it.

94–98

We should add support for multiple languages, eg. by splitting on commas when exporting.

103

Wrong format, schema.org requires this to be a CreativeWork. So the best way to handle this is to ask for an URI, and output:

"isPartOf": {
    "@id": "https://the provided uri",
}
This revision now requires changes to proceed.Nov 5 2019, 2:21 PM

follow the instructions of @vlorentz about L3 and we can continue the work on this tool :-)

This is already done, else you wouldn't be able to read the code :)

Thanks for the info !!
didn't know that :-)

This comment was removed by scatenag.
This comment was removed by scatenag.

description is not an HTML tag, instead you should use a CSS class (eg. <div class="field-description">, and use .field-description { ... } in the .css file)

Also, as these descriptions are added after the input field (which makes sense visually), it means that screen-readers will read the description after the user filled the field (or skipped it), which is not optimal. To fix this, you could add a unique id= to each of the descriptions, and use the aria-describedby attribute on input elements; so screen-readers can locate the description for the field. https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/ARIA_Techniques/Using_the_aria-describedby_attribute

dynamic_form.js
38–41

Still not done; the label and input are inside the p, so it should be:

<p>
    <label for="${personPrefix}_affiliation">Affiliation</label>
    <input type="url" id="${personPrefix}_affiliation" name="${personPrefix}_affiliation"
        placeholder="Department of Computer Science, University of Pisa" />
</p>
index.html
191–240

Could you move this before the author/contributors fieldsets, to optimize space, as they are inline-blocks?

style.css
13

Could you add #fieldsetAdditionalInfo to this list? (same reason as above)

36–38

Don't override the default color for valid input; because if the user uses a dark theme (ie. with text in white-on-black), then this text becomes unreadable (black-on-black)

36–41

Good addition. Could you remove the indentation before the }?

This revision now requires changes to proceed.Nov 21 2019, 11:50 AM
scatenag marked 5 inline comments as done.
  • revision and aria-describedby on description

I hope I did the aria-describedby correctly
(I see no effect ... :-/ ).

Apart from finishing the fieldset with missing array-values...
we are thinking about getting this tool running somewhere and put a link in the SWHAPPE and in the SWHAP guide...

Have you thought about hosting this tool?
Could you host this somewhere?

This revision is now accepted and ready to land.Nov 21 2019, 1:02 PM

Sorry, I missed your comment when reviewing the changes.

I hope I did the aria-describedby correctly

You did :)

(I see no effect ... :-/ ).

It's not supposed to be shown visually, it's an attribute used by some screen readers (look for the "aria-describedby" table here: https://www.powermapper.com/tests/screen-readers/aria/ for a list of supporting screen readers)

Apart from finishing the fieldset with missing array-values...

We can do that in an other diff.

we are thinking about getting this tool running somewhere and put a link in the SWHAPPE and in the SWHAP guide...

Have you thought about hosting this tool?
Could you host this somewhere?

ping @moranegg

@scatenag Could you land this diff? (see https://wiki.softwareheritage.org/wiki/Code_review_in_Phabricator#Landing_your_change_onto_master )

Then rebase/arc diff --update D2351. That will allow me to review it and see only its new changes.

Yes ...but, initially I did understand "ready to land" mean ..
Now..i got this

arc land --revision D2216

BUILDS PASSED Harbormaster builds for the active diff completed successfully.
PUSHING Pushing changes to "origin/master".
git@forge.softwareheritage.org: Permission denied (publickey,keyboard-interactive).
fatal: Could not read from remote repository.

..