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

Fix classes for modWebLink/modSymLink in quick search #16659

Open
wants to merge 1 commit into
base: 3.x
Choose a base branch
from

Conversation

Ruslan-Aleev
Copy link
Collaborator

@Ruslan-Aleev Ruslan-Aleev commented Dec 6, 2024

What does it do?

Fix classes for modWebLink/modSymLink in quick search.
Since links are not exactly resources, we shouldn't apply a template icon to them in search; it's difficult to find the resource you need, especially if the names of the link and resource are the same.
p.s. In the tree, links are already displayed with default icons, not template icons.

Before:
before

After:
after

Why is it needed?

UX: It is easier to understand now which resource is a link.

Related issue(s)/PR(s)

#16644

@Ruslan-Aleev Ruslan-Aleev added type-frontend Issues related to UI/UX issues, mostly about styles and frontend implementations on JavaScript. feature Request about implementing a brand new function or possibility. pr/review-needed Pull request requires review and testing. labels Dec 6, 2024
@Ruslan-Aleev Ruslan-Aleev added this to the v3.0.6 milestone Dec 6, 2024
@opengeek opengeek modified the milestones: v3.0.6, v3.1.0 Dec 9, 2024
@smg6511
Copy link
Collaborator

smg6511 commented Dec 11, 2024

Let me ask folks this: What's an example use-case for applying a Template to a symlink or weblink? My feeling is that it's actually an undesirable inconsistency to apply the custom icon for some resource types and not others, as I would assume the reason to even specify a custom icon is to easily recognize all resources using a particular Template. However, if others agree that applying the icon conditionally is the right thing to do, we should adjust the description for the Manager Icon Class field (which currently states that the icon will apply to “all resources using this template”).

@Ruslan-Aleev
Copy link
Collaborator Author

Ruslan-Aleev commented Dec 11, 2024

For a SymLink , the template icon can be left (although I wouldn't leave it), since the template is used for the SymLink resource, as far as I understand. But for a WebLink, there is no point in setting a template icon at all, it will only confuse (both in the tree and in quick search).

@Mark-H
Copy link
Collaborator

Mark-H commented Dec 11, 2024

Template icons are meant to override the default icon when set. I'm not sure if I understand why you want to undo that override when you could just... not set the override in your site's template(s)?

As long as I don't add a manager icon to the templates, my search is showing as expected:

Scherm­afbeelding 2024-12-11 om 18 47 42

This seems like it would introduce an unnecessary inconsistency between how resources show in the tree and the search.

@Ruslan-Aleev
Copy link
Collaborator Author

Ruslan-Aleev commented Dec 11, 2024

This seems like it would introduce an unnecessary inconsistency between how resources show in the tree and the search.

In the tree, links are displayed with their default icons (always), not the template icon.
And does it make sense to apply a template icon to a link? A link is not a resource, it does not have a template, at least WebLink-resource. And if the link is to another site? Then the template icon has no meaning at all, it only confuses.
What is the use of a template icon for quick search? So that the user goes to the resource, sees that it is a link, and opens the desired resource? Extra steps.

@smg6511
Copy link
Collaborator

smg6511 commented Dec 11, 2024

In the tree, links are displayed with their default icons (always)

I don't think that is the intended behavior and that's the issue that should be solved. I think the point that Mark is making is that you have control over the appearance of custom icons by either not applying a Template at all to your *links, or not specifying a custom icon for the Template(s) that you do apply to *links. That said, why would you assign a Template to either one of the link Resource types anyway (the question I was getting at above re use cases)? Perhaps we're misunderstanding something?

@Mark-H
Copy link
Collaborator

Mark-H commented Dec 11, 2024

In the tree, links are displayed with their default icons (always), not the template icon.

Ah, I overlooked that. Thanks for clarifying.

What would need to be fixed IMO then is the tree, not the search.

As an example of why you might still have a template assigned to weblinks, on MODX.today we use that to have different TVs that affect how the weblink is rendered in the collection of articles. The link itself may not be rendered as a standalone resource, but it may still appear on the site in different ways.

@Ruslan-Aleev
Copy link
Collaborator Author

why would you assign a Template to either one of the link Resource types anyway

I understood what you mean, I didn't think about it earlier, and we didn't understand each other.
MODX has automatic template assignment, so when you create a resource, it will have a template. Of course, you can remove the template when you change the resource type to a link, but this is an unnecessary action in my opinion.

What would need to be fixed IMO then is the tree, not the search.

I don't agree with this, in my opinion it will reduce UX.

@opengeek opengeek modified the milestones: v3.1.0, v3.1.1 Dec 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Request about implementing a brand new function or possibility. pr/review-needed Pull request requires review and testing. type-frontend Issues related to UI/UX issues, mostly about styles and frontend implementations on JavaScript.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants