Skip to content

support custom analyzer#13

Open
TsXor wants to merge 3 commits intoblomstra:main-backup-20231018from
TsXor:custom-analyzer
Open

support custom analyzer#13
TsXor wants to merge 3 commits intoblomstra:main-backup-20231018from
TsXor:custom-analyzer

Conversation

@TsXor
Copy link

@TsXor TsXor commented Sep 24, 2023

#12
added a custom widget class to support custom analyzer, which looks quite primitive
(have corrected the typo in the following picture)
23:45:35
23:45:44

@rafaucau
Copy link

rafaucau commented Sep 27, 2023

@luceos, could you review and merge please?

@luceos
Copy link
Contributor

luceos commented Sep 28, 2023

I'd rather see the dropdown completely removed to make it a free input type text one. This custom component, including its design (sorry), will not make things simpler for people. If you are able to take care of that, then obviously I would have no issue merging.

@TsXor
Copy link
Author

TsXor commented Sep 28, 2023

I'd rather see the dropdown completely removed to make it a free input type text one. This custom component, including its design (sorry), will not make things simpler for people. If you are able to take care of that, then obviously I would have no issue merging.

I thought the dropdown is designed for a reason so I decide to keep it and allow custom string at the same time. As you say so, things have become quite simple.

@rafaucau
Copy link

rafaucau commented Sep 29, 2023

Idea: A simpler way is to add a text field named Custom analyzer and add a help message that if the field is not empty, the custom analyzer will be used instead of the selected language.

    .registerSetting({
      setting: 'blomstra-search.custom-analyzer',
      label: app.translator.trans('blomstra-search.admin.custom-analyzer'),
      help: app.translator.trans('blomstra-search.admin.custom-analyzer-help'),
      type: 'input',
    })

And in PHP something like this:

'type' => $settings->get('blomstra-search.analyzer-language') ?: 'english',

- 'type' => $settings->get('blomstra-search.analyzer-language') ?: 'english',
+ 'type' => $settings->get('blomstra-search.custom-analyzer') ?: $settings->get('blomstra-search.analyzer-language') ?: 'english',

@TsXor
Copy link
Author

TsXor commented Oct 1, 2023

Idea: A simpler way is to add a text field named Custom analyzer and add a help message that if the field is not empty, the custom analyzer will be used instead of the selected language.

implemented in frontend:
19:23:16
@rafaucau @luceos

@luceos
Copy link
Contributor

luceos commented Oct 4, 2023

Although I appreciate the effort and the flexibility of listening to the ideas provided, I do think that multiple controls to manage one setting is going to cause issues.

One of the following would be my preference:

  1. Using only a text field and linking to an article that shows the most common analyzers; this is necessary so that people can see their own locale/language pack reflected in a list.
  2. Showing the dropdown, but adding an option "Custom" that, once selected, will make the Custom analyzer visible.

In terms of UX similarity to Flarum option 2 would be best. In terms of simplicity option 1 would be best.

The current solution however, will guarantee cause confusion and as such raise support workload, so I'm not comfortable merging it in the current state.

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