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

refactoring on scenarios list filter #2688

Merged
merged 2 commits into from
Jun 19, 2024
Merged

refactoring on scenarios list filter #2688

merged 2 commits into from
Jun 19, 2024

Conversation

noodom
Copy link
Contributor

@noodom noodom commented Jun 13, 2024

Bonjour Loïc,

J'ai modifié la proposition en supprimant la fonction avec le code du listener directement dans la balise </script>.
J'ai aussi ajouté le filtre en balise HTML (A voir si la déclaration est à retoucher).


Add a scenarios list filter when adding a scenario in a scenario modification

Description

The PR adds a filter to the scenarios list when the action 'scenario' is added.
Only matching scenarios to the filter (scenario name, scenario group or scenario object parent) are visible in the list.

Suggested changelog entry

add a scenarios list filter in a scenario

Types of changes

  • Bug fix (non-breaking change which fixes)
  • [x ] New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • This change is only breaking for integrators, not for external standards or end-users.
  • Documentation improvement

PR checklist

  • [x ] I have checked there is no other PR open for the same change.
  • [ x] I have read the [La ligne directrice pour contribuer à ce projet / Contribution guidelines for this project).
  • [x ] I grant the project the right to include and distribute the code under the GNU.
  • I have added tests to cover my changes.
  • [ x] I have verified that the code complies with the projects coding standards.
  • [Required for new sniffs] I have added MD documentation for the sniff.

noodom added 2 commits June 13, 2024 12:57
function declaration removed
concole.log removed
@zoic21
Copy link
Contributor

zoic21 commented Jun 13, 2024

Bonjour,
Merci pour la modification par contre je pense il va y avoir un soucis avec le const filter qui sera different pour chaque bloc de selection de scénario mais avec une const ca va poser soucis peut etre plutot faire :

document.querySelector('.expressionAttr[data-uid="#uid#"][data-l1key="options"][data-l2key="filter"]').addEventListener('keyup', function(event) {

Pour les autres const dans la fonction je pense il faudrait plutot des let.

@noodom
Copy link
Contributor Author

noodom commented Jun 13, 2024

Pour moi, ça ne pose pas de problème.
La portée de la déclaration du const est interne à la callback success.

J'avais d'ailleurs testé avec plusieurs actions 'scenario' et ça ne remonte aucun souci.
Pour moi, c'est plus sûr (et sain) de laisser les const là où c'est suffisant (un let ne changera pas la portée et les variables déclarées n'ont pas besoin d'être modifiées).

@zoic21
Copy link
Contributor

zoic21 commented Jun 14, 2024

Effectivement j'ai rien dit const et let ont la meme portée. C'est donc tout bon pour moi

@zoic21 zoic21 merged commit 4aad688 into jeedom:alpha Jun 19, 2024
3 checks passed
@noodom noodom deleted the patch-23 branch July 30, 2024 17:52
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.

3 participants