Skip to content

Conversation

@franklupo
Copy link
Contributor

I added Search for NavMenu.

image

public NavLinkMatch Match { get; init; } = NavLinkMatch.Prefix;
public Icon Icon { get; init; } = new Icons.Regular.Size20.Document();
public bool Visible { get; set; }
public IEnumerable<string> Tags { get; set; } = [];
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Possible to add tags for search

item.Visible = string.IsNullOrEmpty(_term)
? true
: item.Title.Contains(_term, StringComparison.OrdinalIgnoreCase)
|| item.Tags.Any(a => a.Contains(_term, StringComparison.OrdinalIgnoreCase));
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Filter

@vnbaaij
Copy link
Collaborator

vnbaaij commented Jun 29, 2024

What is the difference between this and the search top right?

Like the tags idea though. That could be useful.

@franklupo
Copy link
Contributor Author

I like it above the menu, and I think the mobile experience is better.

@vnbaaij
Copy link
Collaborator

vnbaaij commented Jun 29, 2024

So there is no real difference between the two?

Could you include a video of how it works so we don't have to check out the branch ourselves? (tip, we use Peek to create animated gifs. Bit lighter then mp4 files)

I agree on the mobile experience argument. But if we are going to do this, then this needs to be a complete PR that also removes the other search option. I see no point in having both.

@franklupo
Copy link
Contributor Author

peek_1

@vnbaaij
Copy link
Collaborator

vnbaaij commented Jun 29, 2024

Hmm, I would expect it to work more like the current search. So typing 'serv' would give you:
image

I think when showing a search result, having the groups as entry point is not desirable. Just show the results. But then again, it does not give you the context of in what group an item lives anymore. I think adding this this will always introduce trade-offs.

Maybe it is better to not mix these two functionalities (search and navmenu) but just to move existing search to show above the nav menu (so we have a better mobile experience) and integrate in this existing search the tags functionality you added (to get better results) and not filter the menu.

@franklupo
Copy link
Contributor Author

the idea is to help the user know where he is

@dvoituron
Copy link
Collaborator

I don't see the advantage of adding functionality that already exists on the website.

On the other hand, I prefer this location (above the navigation)... but then several points need to be reviewed:

  1. It's not a "Search" but a "Filter".
  2. Add Immediate="true" ImmediateDelay="100".
  3. Groups must be automatically Expanded when filtering is applied (this is currently not the case), and using the "normal" state when no filter is applied.
  4. This filter zone is not responsive, which contradicts the advantage of using it on Mobile.
  5. I don't agree with changing the data model, adding Visible to handle a UI feature. We need to find another way, via a method that returns the list of useful navMenuItems for example. And put this code in a DemoNavMenu.razor.cs file and not in @code section.
  6. Who defines tag content? What criteria? It's great to have a feature, but it needs to be used.

@dvoituron
Copy link
Collaborator

the idea is to help the user know where he is

You have the title of the page and the indication in the menu on the left for that.

@vnbaaij
Copy link
Collaborator

vnbaaij commented Jun 30, 2024

I don't see the advantage of adding functionality that already exists on the website.

Agreed, there should only be one

On the other hand, I prefer this location (above the navigation)...

Agreed

  1. It's not a "Search" but a "Filter".

I think and end user does not make that distinction.

  1. Add Immediate="true" ImmediateDelay="100".

Agreed

  1. Groups must be automatically Expanded when filtering is applied (this is currently not the case), and using the "normal" state when no filter is applied.

Agreed, but this wil be difficult to achieve as we don't have any notion of parent/child in the menu data structures. Should we add that?

  1. This filter zone is not responsive, which contradicts the advantage of using it on Mobile.
    Agreed
  1. I don't agree with changing the data model, adding Visible to handle a UI feature. We need to find another way, via a method that returns the list of useful navMenuItems for example. And put this code in a DemoNavMenu.razor.cs file and not in @code section.

Agreed

  1. Who defines tag content? What criteria? It's great to have a feature, but it needs to be used.

We could perhaps better use the Data property that we already have for this tagging info (every FluentComponentBase derived component has Data parameter). Then the dev can just add that either in a declarative ay or when building the Items collection.

Maybe we should not try to integrate this into the NavMenu but make it a separate component akin to the FluentPaginator/FluentDataGrid. If you want to use it, you set a parameter on the NavMenu and it will be used.

@vnbaaij
Copy link
Collaborator

vnbaaij commented Nov 19, 2024

Closing this as there has been no activity for a while.

@vnbaaij vnbaaij closed this Nov 19, 2024
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