Search

[dcunhas]

I added a search bar to the URL and Substitution areas. Because I'm not a web designer, the added UI looks a little rough but it is functional. Please make some suggestions on changes necessary before this can be merged. This should resolve issue #134.

[Woundorf]

I will need a few days (or weeks) to review this appropriately and test it, but on a superficial level I see that there is inconsistent indentation, mixing spaces and tabs. Please convert them all to the appropriate number of spaces.

[dcunhas]

Sorry about that, didn't realize my editor was not configured correctly. The space issue should be fixed now. Take your time reviewing, I'm in no rush to have it merged.

[Woundorf]

Hi, I have now had some time to review it in detail and test it. I have found a few issues that would need to be resolved. I will also add a few comments to specific lines for other minor things.

1. Sometimes the last empty row that allows to add new elements can be duplicated (or triplicated, etc.)

Steps to reproduce:

1. Edit a group with some entries (it doesn't matter if URLs or substitutions, it happens on both places).
2. Write something in the search box allowing some element to be visible.
3. Start to edit one of the visible items.
4. Clear the search box.

With these steps a new empty row is added at the end. It is added by this fragment in groupeditor.js (this one for urls and a similar one for substitutions):

      let isLast = isLastRow(params);
      let isEmpty = params.value === "";

      if (!isLast && isEmpty) {
        params.api.updateRowData({ remove: [params.data] });
      }
      else if (isLast && !isEmpty) {
        params.api.updateRowData({ add: [{ url: "" }] });
      }

The conditions could be adjusted to take into account the search (or maybe there's a better solution).

2. Move actions in the substitutions tab don't behave well when a filter is active

Unless you can think of a way of making them work, they could be disabled when the filter is active.

[dcunhas]

I think I fixed everything you mentioned. Notably for the issue with the movement buttons, I just disabled them while searching. I can conceive of (rare) situations where one might want to move an entry's position while searching, but the logic and methodology of that seems nontrivial to me, so that could be considered a possible future feature.

[dcunhas]

I think I fixed everything, let me know if there's anything more. On another note, have you considered using something like prettier in a pre-commit hook to automate code style compliance?

[Woundorf]

Thanks, now everything is ok.

On another note, have you considered using something like prettier in a pre-commit hook to automate code style compliance?

I didn't know this particular tool, but also since I don't usually receive contributions I haven't had any need for it.