Skip to content

Conversation

@FlorentDotMe
Copy link
Contributor

Description

Every time a user performs a search for a value that is not an asset tag, a warning message will show up mentioning the assset wasn't found. If such use of the search field is intentional, it would be good to allow the user to permanently dismiss the warning message.

Note: for the search feature, refer to PR #15217.

What changes
This PR gives the possibility to permanently dismiss the warning message shown when an asset tag doesn't exist.

Current proposition:
image

Remaining tasks

  • Improve the design: any suggestion is welcome.
  • Is it ok to store the user choice in the localStorage? Or better in a cookie? Or in database (...)?

Type of change

  • New feature (non-breaking change which adds functionality)

How Has This Been Tested?

  1. Search for a random string in the "Lookup for Asset Tag" search box
  2. Confirm the warning message shows up
  3. Click on "Dismiss permanently"
  4. Perform another search in the "Lookup for Asset Tag" search box
  5. The warning message should not show up anymore

Checklist:

@what-the-diff
Copy link

what-the-diff bot commented Aug 6, 2024

PR Summary

  • Change in Warning Method in Assets Controller
    The warning signaling method in Assets Controller has been modified. Initially, it was denoted as 'warning', it is now rephrased as 'search_warning'. This change contributes to clearer communication within code and enhances code readability.

  • Addition of a New Function in the Javascript File
    A new function has been introduced into the Javascript file. The main role of this function is to deal with search warning messages and dismiss them permanently. It will improve the system's adeptness at handling warnings and improve user experience with cleaner UIs, by removing persistent warning messages.

  • Update in Translation String in General PHP
    A newer translation string 'notification_search_warning' has been incorporated into the general PHP. This update sets rules for the software to 'understand' and translate this specific term in various sequences. It primarily enhances localization and accessibility, allowing users from different regions or language preferences to comprehend the warning messages.

  • Addition of a New Section in Notifications Blade PHP
    A new part has been incorporated into Notifications Blade PHP. This section is crucial because it displays the search warning message and offers an option to dismiss it for good. This change aids in providing users with better control of their visual interface, enabling them to clear unnecessary or recurring warning messages to establish a clutter-free workspace.

@FlorentDotMe FlorentDotMe force-pushed the fix_search-warning-dismiss branch from 13a2e91 to 064745a Compare August 7, 2024 13:36
@FlorentDotMe
Copy link
Contributor Author

New design introduced in the last commit:
image

With mouseover:
image

@FlorentDotMe FlorentDotMe force-pushed the fix_search-warning-dismiss branch 2 times, most recently from 0a67fb3 to c008f2e Compare August 7, 2024 14:26
@FlorentDotMe FlorentDotMe marked this pull request as ready for review August 7, 2024 14:43
@FlorentDotMe FlorentDotMe requested a review from snipe as a code owner August 7, 2024 14:43
@snipe
Copy link
Member

snipe commented Aug 7, 2024

I'm not 100% sure I see the use case here, but I think we'd want that dismiss link to be white so it's clearer.

@FlorentDotMe FlorentDotMe force-pushed the fix_search-warning-dismiss branch from c008f2e to 18991c7 Compare August 8, 2024 14:57
@FlorentDotMe
Copy link
Contributor Author

With the design adjustment:
image

@FlorentDotMe
Copy link
Contributor Author

I'm not 100% sure I see the use case here, but I think we'd want that dismiss link to be white so it's clearer.

And to clarify the purpose, in our organization we are used to look for a specific asset tag, but also for other attributes (serial number, manufacturer, model, ...).

As per our discussion in PR #15217, you're right, it makes sense to show the warning message. But if searching for other attributes is part of the user workflow, I think it would make sense to allow the user to ignore the warning message.

To be honest, I'm not 100% convinced of the adhesion it would get from other users. If you wish, we can put this PR on hold/draft until it receives more attention from the community.

@FlorentDotMe FlorentDotMe marked this pull request as draft August 8, 2024 15:57
@FlorentDotMe
Copy link
Contributor Author

I also have this variant with a dropdown menu, but this piece of code is not yet ready.
image

@brianhoganm
Copy link

Can I ask why??? We have several admins at different locations that they support. Sometimes assets get Transferred from one location to another. If an admin only has access to a certain company and they come across an asset that is not there's, it would be nice if the message would say "this asset [asset number] belongs to [company name]. Instead of permanently dismissing this message all together, can we incorporate some logic scenario messages if multi company support is enabled and for admins who only have access to certain companies? This would be a huge benefit to ensure that assets get returned to their proper locations without always having to puck up the phone and ask..."hey do you have this asset???"

@snipe
Copy link
Member

snipe commented Aug 8, 2024

@brianhoganm calm down with the question marks lol - everybody has different workflows. I definitely would not want to disclose the name of the company the asset belongs to (MSPs might not want their other customer names exposed) but we would only do this on an individual level if we even go down this path.

@brianhoganm
Copy link

@brianhoganm calm down with the question marks lol - everybody has different workflows. I definitely would not want to disclose the name of the company the asset belongs to (MSPs might not want their other customer names exposed) but we would only do this on an individual level if we even go down this path.

I would make this as an option to enable this in settings

@FlorentDotMe FlorentDotMe mentioned this pull request Aug 20, 2024
2 tasks
@FlorentDotMe FlorentDotMe force-pushed the fix_search-warning-dismiss branch from 15a932f to a39d687 Compare November 26, 2024 13:37
@FlorentDotMe
Copy link
Contributor Author

The last commit a39d687 makes this feature available as a profile setting, instead of using the browser local storage.

@FlorentDotMe FlorentDotMe marked this pull request as ready for review November 26, 2024 13:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants