-
Notifications
You must be signed in to change notification settings - Fork 3
Add provenance
and comments
Fields
#252
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
Conversation
…ance` and `comments`. If you open a project that predates the fields, the fields are hidden by default. Use the Template Editor to show them.
Nice!! Looking good so far! A few minor notes:
More To DoBL Adding more to do items to this comment so that it's all in one place.
|
If we want to change the default text for provenance and comments, should we edit template-schema (in a separate branch and then merge request)? |
You'll want to edit the |
Makes sense and we can wait until this is locked down but figured I’d ask! Thanks!! |
…(and prevent wrapping). Hover help still works fine.
…js`. (Will be moot once #255 is fixed)
…schema.js` using CLI `ncRegenerateDefaultTemplate()` command
Some changes to the default template. Excerpts from the wiki: https://github.com/netcreateorg/netcreate-2018/wiki/Using-Templates/ Templates and New Projects
|
Testing
|
…ts so it's more consistent with the rest of the fields.
Quick thought - is there an easy way, that isn't a waste of time, to modify the comment filter so that we can say something like "if it is not empty"? Long-term that'll be a badge, but checking for non-empty or empty fields feels useful. But not if it is a distraction. @benloh |
Standalone appears to work, though I'll keep banging on it. |
The provenance and comments fields appear to show up in the table view but clicking on them does not sort the data the way the other tabs do. |
The notes field is now a bit narrow ... not sure how we want to solve that long-term, though? I had been thinking a full table module / plugin would be nice to let people re-size columns but easier said than done I presume? |
@jdanish I think we can easily add a new test for strings.
It seems to work for me. Are you sure it's broken? Can you tell me how to reproduce or show me a video?
I didn't really touch the styling on the table at all. We might consider making the tables the full width of the browser to give them more room. We could also style the text a little more (e.g. Provenance can be smaller. The text in general could probably be smaller. I didn't want to touch the styling without checking in with you first. Do you want to take a crack at it and style it as I see fit? Or did you want to talk through the changes?
This really depends on whether we can find the right library. In the past, we had issues with libraries not working well with React, or being a pain in the tuchus to implement. If this is significant enough of an issue, add it to the issues db and we can prioritize. |
The provenance and comment sorting do appear to work now. Prior issue may have been an artifact of sparse population in the sense that it worked but I couldn't tell. Re: styling, do what you need, but the big overhaul should wait until we have more change done and should do a round together at that time. Re: table ... let's revisit when we look at the long-term timeline etc. |
Added "is empty" and "is not empty" |
… text, moved Comments to end.
Provenance and Comment now sort properly for v1.3-era projects that have been only partially migrated to the current version. The issue was that |
@jdanish this ended up expanding to many other issues/fixes, but I think we should be able to merge and close this now? We'll handle the other issues in another branch/merge? |
@benloh Ready to merge! Only last minor thing is that the yellow on the comments on nodes table (awesome) isn't also on edges. I'd say merge it once that is in, though! |
Oh, one minor catch @benloh - provenance needs to be slightly wider to keep the arrow visible. ![]() |
|
Merging and closing. |
This pull request adds the
provenance
andcomments
fields.See the issues for details on implementation.
Both fields have now been added to the base
template-schema.js
.We can now also generate a new default template JSON from the
template-schema.js
.Both fields can be added to filters (this came for free with our template-schema implementation).
If you open a project that did not have these fields defined, the project will gracefully open with no errors and by default hide the fields. If you want them on, edit the template to unhide them.