-
Notifications
You must be signed in to change notification settings - Fork 26
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
Button default options #1246
base: next
Are you sure you want to change the base?
Button default options #1246
Changes from 32 commits
fde57fe
82e0566
8be13d1
7d6a448
aa16282
5b2a613
5bdce71
2746eea
6102c0e
4ab2019
46b13f9
f19f11e
93b738d
dd71866
4e35c6d
80ae099
6f0d21a
0a88b15
b70f52f
2a9c5b9
c80c8bf
7606fea
5b28cf3
b36486c
c5888c1
c56a3d8
9acf4b9
dcd3a37
a458db7
a3e85a9
d96c5bf
117b41c
884e40e
b0e96e2
239b73c
a3688d2
6f72389
c7c7a6f
110860e
3ccb8bd
9c60f08
0bc962c
f85c57b
5872918
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -203,6 +203,50 @@ export class SearchSettingsComponent implements OnInit { | |||||||||||||||||||
}); | ||||||||||||||||||||
} | ||||||||||||||||||||
|
||||||||||||||||||||
/** | ||||||||||||||||||||
* Triggered when the default options is clicked | ||||||||||||||||||||
* Only CheckBox type will change | ||||||||||||||||||||
* Default Button of a specified setting | ||||||||||||||||||||
* @internal | ||||||||||||||||||||
*/ | ||||||||||||||||||||
checkDefaultOptions( | ||||||||||||||||||||
event, | ||||||||||||||||||||
pelord marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||
source: SearchSource, | ||||||||||||||||||||
setting: SearchSourceSettings | ||||||||||||||||||||
) { | ||||||||||||||||||||
event.stopPropagation(); | ||||||||||||||||||||
setting.allEnabled = true; | ||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Pourquoi mettre le allEnabled à vrai? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sinon, ça crée un état où le bouton de sélection par défaut sélectionnait tout lorsque tout était désélectionné. |
||||||||||||||||||||
this.checkUncheckAll(event, source, setting); | ||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Pourquoi faire un uncheck si on repasse tous les settings ci-dessous. Surtout qu'on va générer deux searchSourceChange.emit() There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Dépendamment de la valeur du bouton de tout sélectionner, un état faisait que le bouton de sélection par défaut ne fonctionnait pas. Ça "reset" en quelque sorte. |
||||||||||||||||||||
source.getDefaultOptions().settings.map((defaultSetting) => { | ||||||||||||||||||||
if (defaultSetting.title === setting.title) { | ||||||||||||||||||||
setting.values.map((value, index) => { | ||||||||||||||||||||
pelord marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||
value.enabled = defaultSetting.values[index].enabled; | ||||||||||||||||||||
}); | ||||||||||||||||||||
} | ||||||||||||||||||||
}); | ||||||||||||||||||||
Comment on lines
+248
to
+254
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. J'ai l'impression qu'on pourrait optimiser et améliorer la lisibilité? Je n'ai pas testé!
Suggested change
|
||||||||||||||||||||
this.searchSourceChange.emit(source); | ||||||||||||||||||||
} | ||||||||||||||||||||
|
||||||||||||||||||||
/** | ||||||||||||||||||||
* Triggered when the default option is clicked in the specified source | ||||||||||||||||||||
* @internal | ||||||||||||||||||||
*/ | ||||||||||||||||||||
checkMenuDefaultOptions(event, source: SearchSource) { | ||||||||||||||||||||
event.stopPropagation(); | ||||||||||||||||||||
source.settings.map((setting, index) => { | ||||||||||||||||||||
setting.allEnabled = true; | ||||||||||||||||||||
this.checkUncheckAll(event, source, setting); | ||||||||||||||||||||
|
||||||||||||||||||||
setting.values.map((value, settingsIndex) => { | ||||||||||||||||||||
value.enabled = | ||||||||||||||||||||
source.getDefaultOptions(true).settings[index].values[ | ||||||||||||||||||||
settingsIndex | ||||||||||||||||||||
].enabled; | ||||||||||||||||||||
}); | ||||||||||||||||||||
}); | ||||||||||||||||||||
this.searchSourceChange.emit(source); | ||||||||||||||||||||
} | ||||||||||||||||||||
|
||||||||||||||||||||
/** | ||||||||||||||||||||
* Triggered when a setting is checked (radiobutton style) | ||||||||||||||||||||
* @internal | ||||||||||||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -49,7 +49,7 @@ export class CadastreSearchSource extends SearchSource implements TextSearch { | |
/* | ||
* Source : https://wiki.openstreetmap.org/wiki/Key:amenity | ||
*/ | ||
protected getDefaultOptions(): SearchSourceOptions { | ||
getDefaultOptions(): SearchSourceOptions { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Pourquoi enlever le protected? |
||
return { | ||
title: 'Cadastre (Québec)', | ||
searchUrl: 'https://carto.cptaq.gouv.qc.ca/php/find_lot_v1.php?' | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -115,16 +115,15 @@ export class IChercheSearchSource extends SearchSource implements TextSearch { | |
return IChercheSearchSource.type; | ||
} | ||
|
||
protected getDefaultOptions(): SearchSourceOptions { | ||
getDefaultOptions(forceReset?: Boolean): SearchSourceOptions { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Erreur de typage |
||
const limit = | ||
this.options.params && this.options.params.limit | ||
? Number(this.options.params.limit) | ||
!forceReset && this.options.params && this.options.params.limit | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Pourquoi le forceReset, dans quel scénario on pourrait avoir des résultats par défaut qui sont différent? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. C'est pour le bouton qui remet tous les dépendances par défaut. Ce bouton n'a pas les settings dans son container. Sans le forceReset, les valeurs par défaut des radio button changeaient selon l'option sélectionnée. |
||
? Number(this.options.params.limit) | ||
alecarn marked this conversation as resolved.
Show resolved
Hide resolved
|
||
: undefined; | ||
const ecmax = | ||
this.options.params && this.options.params.ecmax | ||
!forceReset && this.options.params && this.options.params.ecmax | ||
? Number(this.options.params.ecmax) | ||
: undefined; | ||
|
||
const types = this.options.params?.type | ||
? this.options.params.type.replace(/\s/g, '').toLowerCase().split(',') | ||
: [ | ||
|
@@ -715,9 +714,9 @@ export class IChercheReverseSearchSource | |
return IChercheReverseSearchSource.type; | ||
} | ||
|
||
protected getDefaultOptions(): SearchSourceOptions { | ||
getDefaultOptions(forceReset?: Boolean): SearchSourceOptions { | ||
const types = | ||
this.options.params && this.options.params.type | ||
!forceReset && this.options.params && this.options.params.type | ||
? this.options.params.type.replace(/\s/g, '').toLowerCase().split(',') | ||
: ['adresses', 'municipalites', 'mrc', 'regadmin']; | ||
|
||
|
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.
En général avec Material design, on essaie de respecter les multiples de 4 ou au moins en nombre paires pour nos espacements (padding/margin).
Voir le Material measurements
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.
Je me base sur des éléments déjà existants comme le bouton de tout sélectionner qui utilise ce multiple
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.
Je crois qu'il faut se conformer ici