-
Notifications
You must be signed in to change notification settings - Fork 9
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
Allow toggling samples & regions in add things step UI #1378 #1343
Conversation
660327a
to
99eec1f
Compare
99eec1f
to
5523702
Compare
const tilesBelongingToManifest = this.tilesBelongingToManifest(self.card.tiles(), self.canvases()); | ||
const tilesBelongingToManifest = this.tilesBelongingToManifest(self.card.tiles()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unrelated cleanup re #1304; this function no longer takes two args, missed it before merge.
) && (self.includeSamples() || !result.text.includes(sampleSubstring)) | ||
&& (self.includeAnalysisAreas() || !result.text.includes(regionSubstring)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I still found substring matching necessary to filter out children from the select2 widget, as I don't have access to concepts/tiles at that point. Open to other ideas.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we could do something like this on the Arches side: https://github.com/archesproject/arches/pull/10216/files
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think the graphid will help, because both samples and their parents will share the same graph id for physical thing. AFAICT I would need to drill in to the tile data along the lines of what I did in the middle of the long sort function added in this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just tested it out, and this is all I get back with archesproject/arches#10216:
{
"context": "",
"context_label": "Physical Thing - Name",
"graphid": "9519cb4f-b25b-11e9-8c7b-a4d18cec433a",
"id": 1,
"text": "qwe [Sample of Alabastron (2003.192)]",
"type": "term",
"value": "qwe [Sample of Alabastron (2003.192)]"
},
{
"context": "",
"context_label": "Physical Thing - Name",
"graphid": "9519cb4f-b25b-11e9-8c7b-a4d18cec433a",
"id": 2,
"text": "qwe [Sample Area of Alabastron (2003.192)]",
"type": "term",
"value": "qwe [Sample Area of Alabastron (2003.192)]"
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see - right, it's the distinction between types of physical things that is needed, not resource models.
@chiatt ready for a look! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great. I think we can address the issue with filtering the dropdown results once this work is completed: #1435
Closes #1378
Demo