-
Notifications
You must be signed in to change notification settings - Fork 2
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
Preselections #2
Comments
I think we should have two outputs. First an output for URL should never include preselections, because that would mean they would end up in URL, be editable by end-user and then be incorrectly used as a source of truth to reload the search. So end-user would have a too easy way to bypass readonly preselections. And a second output for GraphQL (or other search backends) must include the contexts in order to do a proper search. That means that the developer must provide preselections in addition to whatever is coming from URL. And applying preselections to each group is error prone for the developer. So instead I suggest to have two inputs too, one for selection and one for preselections. That way natural-search can easily identify and apply contexts to each (newly created) group automatically. Something like: <natural-search
[selections]="userSelections"
[preselections]="preSelections"
[configurations]="allConfigurations"
(selectionChange)="updateUrl($event)"
(selectionWithPreselectionChange)="updateFilter($event)"
> Then methods would be similar to: updateUrl(selection) {
const url= toUrl(selection);
}
updateFilter(selectionWithContext) {
const graphqlSelection = toGraphQLDoctrineFilter(this.allConfigurations, selectionWithContext);
} If we choose to have two inputs, then I would suggest to also have two distinct types. Something like export interface GroupPreSelections extends Array<PreSelection> {
}
export interface PreSelection {
selection: Selection;
visible: boolean = true;
forced: boolean = true;
readonly: boolean = true;
} Notice that it is |
Is this really breaking ? contexts (input and output) are optional. |
You're right, this is probably not breaking, only new optional stuff. |
Also I would avoid the term "context" which tends to be a bit overused in our projects and instead try to have something more semantic such as "forcedSelection" or "predefinedSelection". |
I vote for preselections. |
Natural-search can be initialised with
selections
(inputs
). Those selections may be provided by the user and areuser selections
(e.g a previous search that has been stored in the url).The developper may want to add his selections, whether the user provides it's own or not. Those
developper selections
arecontexts (selections)
.What is a preselection ?
It's a regular
user selection
with additional attributes, see #3 .Those additional attributes limit the
input
manipulation by the end user (like edit or remove). Without those additional attributes the user has the same liberty to manipulate theselections
as he would have by adding them himself. In this case, thepreselections
are justuser selections
.Those
preselections
need to be applied on each group automatically (to prevent user to bypass them simply by adding a new group with other filters).In order to send selections to natural-search, we need to provide a matching configuration. I suggest to provide contexts and configurations in the same actual inputs :
Here are the reasons for that :
With a same source of configurations, all the
user selections
created from visible configurations (see configuration settings #3) will have the same interaction rules.But
preselections
provided to natural-search by the parent component have additional attributes. If those attributes where moved into configurations, all the contexts would share the same configuration and e.g be all "readonly" or "invisible" for field "xyz".Opened question
preselection
as it is)preselections
intouser selections
) ?Two solutions only imo :
1) Output without preselections :
The output doesn't include
preselections
. The developper has to complete the output with contexts he gave in input (in some way, he should already have them in a variable). It's a bit "dangerous" as he needs to do it correctly, applying the contexts by himself to each group of the output.In some way, it's what we do with QueryVariablesManager (but we don't send preselections to natural-search).
When reinitialising natural-search from last output, he can simply and blindly add his contexts to the list of selections.
2) Output with preselections : good if no need to reinitialise the search from last output
The output includes the context as they are (with extra fields), maybe with an another additional helper field "{preselection: true}" to help the developper to identify them. Developer needs to identify preselections to splice them and add new ones if needed.
When reinitialising natural-search from last output, the developper needs to identify the contexts given in it and if needed remove them to add new updated contexts.
Maybe a flag to let the developer chose between those options would be nice...
@PowerKiKi Any other solution ?
The text was updated successfully, but these errors were encountered: