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

Add new fields to metadata modal #5016

Merged
merged 9 commits into from
Aug 17, 2015
Merged

Conversation

viddo
Copy link
Contributor

@viddo viddo commented Aug 13, 2015

Fixes #4803

While I didn't have any mockups this time I simply added the new fields between desc and tags, i.e.:

Empty values/placeholders:
screen shot 2015-08-13 at 16 22 35

Licence dropdown:
screen shot 2015-08-13 at 16 57 18

@saleiva @josecruz OK? Or did you have another idae on how it should look?
@csobier and @brittanymicek for copy (see the placeholder texts above). also notice the "-" for the non-selected license perhaps we should have some text there instead?

@javierarce can you do the code review, please?

@javierarce
Copy link
Contributor

👍 Looks good to me.

@saleiva
Copy link
Contributor

saleiva commented Aug 13, 2015

That icon on the dropdown should be the default one.
Also I'd not repeat 3 times the md thingym I propose we leave it just under
the description
On Aug 13, 2015 5:31 PM, "Javier Arce" notifications@github.com wrote:

[image: 👍] Looks good to me.


Reply to this email directly or view it on GitHub
#5016 (comment).

@josecruz
Copy link
Contributor

What's the meaning of this right icon?

screen shot 2015-08-13 at 18 47 13

@csobier
Copy link
Contributor

csobier commented Aug 13, 2015

I can suggest some text for the License field once we know what the icon indicates? Is it a tooltip? It it mandatory? Otherwise, here is my best guess:
"Select the type of license required for copying, distributing, or modifying this dataset"

Additionally, I would keep the field descriptions consistent- either all statements or all questions. Typically, product UI field descriptions are statements so the Source description should say something like "Enter the source of the data"

Nicklas Gummesson added 4 commits August 14, 2015 15:22
@viddo
Copy link
Contributor Author

viddo commented Aug 14, 2015

The icon is a mistake, I suspect the change of the icons recently (should be the default dropdown/chevron icon), fixed now:

screen shot 2015-08-14 at 15 27 49

@csobier It's a standard dropdown (see new screenshot above). It's value is optional (thus the "-" option at the top for, which I was asking if we should replace that with some text instead? Apart from that, updated the placeholders, I'll keep that guideline in mind for the next forms I'll do 👍

@saleiva
Copy link
Contributor

saleiva commented Aug 14, 2015

The margin between the inputs should be the same than between the first and
the second one.
On Aug 14, 2015 3:31 PM, "Nicklas Gummesson" notifications@github.com
wrote:

The icon is a mistake, I suspect the change of the icons recently (should
be the default dropdown/chevron icon), fixed now:

[image: screen shot 2015-08-14 at 15 27 49]
https://cloud.githubusercontent.com/assets/978461/9274852/6067ef30-4299-11e5-9f23-20bc20692259.png

@csobier https://github.com/csobier It's a standard dropdown (see new
screenshot above). It's value is optional (thus the "-" option at the top
for, which I was asking if we should replace that with some text instead?
Apart from that, updated the placeholders, I'll keep that guideline in mind
for the next forms I'll do [image: 👍]


Reply to this email directly or view it on GitHub
#5016 (comment).

@viddo
Copy link
Contributor Author

viddo commented Aug 14, 2015

screen shot 2015-08-14 at 17 25 48

@saleiva
Copy link
Contributor

saleiva commented Aug 14, 2015

🇪🇸
On Aug 14, 2015 5:27 PM, "Nicklas Gummesson" notifications@github.com
wrote:

[image: screen shot 2015-08-14 at 17 25 48]
https://cloud.githubusercontent.com/assets/978461/9277328/9e61c71a-42a9-11e5-9d8a-9cae239fb685.png


Reply to this email directly or view it on GitHub
#5016 (comment).

viddo added a commit that referenced this pull request Aug 17, 2015
@viddo viddo merged commit 0735d21 into master Aug 17, 2015
@viddo viddo deleted the 4803-add-metadata-license-etc branch August 17, 2015 07:35
@viddo
Copy link
Contributor Author

viddo commented Aug 17, 2015

I'll go ahead and merge this, if you want any additional changes just create a new issue and assign me :)

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