-
Notifications
You must be signed in to change notification settings - Fork 12
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
feat(SFINT-4388) no results text when no user actions #106
Conversation
UserActions: 'User Actions', | ||
UserActions_no_actions_title: 'No actions available for this user', | ||
UserActions_no_actions_causes_title: 'Potential causes', | ||
UserActions_no_actions_cause_not_enabled: 'User actions are not enabled for your organization', | ||
UserActions_no_actions_cause_not_associated: 'There are no user actions associated with the user', | ||
UserActions_no_actions_cause_case_too_old: 'The case is too old to detect related actions', | ||
UserActions_no_actions_contact_admin: 'Contact your administrator for help', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@acarpentier01 These are the strings. you can check on the screenshots where they are used.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! 👍
@@ -1,7 +1,13 @@ | |||
import { Translation, Language } from '../../utils/translation'; | |||
|
|||
Translation.register(Language.English, { | |||
UserActions_no_actions: 'No actions available for this user', | |||
UserActions: 'User Actions', | |||
UserActions_no_actions_title: 'No actions available for this user', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
UserActions_no_actions_title: 'No actions available for this user', | |
UserActions_no_actions_title: 'No actions available for this user.', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I should've specified :D the .
is added but elsewhere hehe thanks for confirming I need a .
though!
UserActions: 'User Actions', | ||
UserActions_no_actions_title: 'No actions available for this user', | ||
UserActions_no_actions_causes_title: 'Potential causes', | ||
UserActions_no_actions_cause_not_enabled: 'User actions are not enabled for your organization', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
UserActions_no_actions_cause_not_enabled: 'User actions are not enabled for your organization', | |
UserActions_no_actions_cause_not_enabled: 'User actions are not enabled for your organization.', |
UserActions_no_actions_title: 'No actions available for this user', | ||
UserActions_no_actions_causes_title: 'Potential causes', | ||
UserActions_no_actions_cause_not_enabled: 'User actions are not enabled for your organization', | ||
UserActions_no_actions_cause_not_associated: 'There are no user actions associated with the user', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
UserActions_no_actions_cause_not_associated: 'There are no user actions associated with the user', | |
UserActions_no_actions_cause_not_associated: 'There are no user actions associated with the user.', |
UserActions_no_actions_causes_title: 'Potential causes', | ||
UserActions_no_actions_cause_not_enabled: 'User actions are not enabled for your organization', | ||
UserActions_no_actions_cause_not_associated: 'There are no user actions associated with the user', | ||
UserActions_no_actions_cause_case_too_old: 'The case is too old to detect related actions', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
UserActions_no_actions_cause_case_too_old: 'The case is too old to detect related actions', | |
UserActions_no_actions_cause_case_too_old: 'The case is too old to detect related actions.', |
UserActions_no_actions_cause_not_enabled: 'User actions are not enabled for your organization', | ||
UserActions_no_actions_cause_not_associated: 'There are no user actions associated with the user', | ||
UserActions_no_actions_cause_case_too_old: 'The case is too old to detect related actions', | ||
UserActions_no_actions_contact_admin: 'Contact your administrator for help', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
UserActions_no_actions_contact_admin: 'Contact your administrator for help', | |
UserActions_no_actions_contact_admin: 'Contact your administrator for help.', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All good for me! Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only added a few minor comments. It looks good to me.
@@ -318,16 +317,33 @@ export class UserActions extends Component { | |||
private renderNoActions() { | |||
const element = document.createElement('div'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For a moment, I was confused to see these two lines together. Didn't notice the this.
at first.
element.innerHTML = // the whole no actions markup...
this.element.innerHTML = '';
I'd suggest to rename the element
variable to emptySpace
or something like that to avoid confusion with this.element
. Feel free to keep the code as is if you prefer though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yea good idea, I was confused about it too (not my code the element part ;) )
element.innerHTML = ` | ||
<div class="coveo-user-actions-title">${l(UserActions.ID)}</div> | ||
<div>${l(UserActions.ID + '_no_actions_title')}.</div> | ||
<br/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of adding <br />
tags like this, you could add a class like coveo-no-actions-section
to add the top margin you want.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wrote a comment but it disappeared :(
Basically the thing we are displaying I consider more as a 1 paragraph than multiple HTML elements. What do you think? It felt more like a line break, than a margin between elements.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tes en feu
New way of explaining why there are no user actions visible.
Where the user actions are not enabled (404 response) :
![image](https://user-images.githubusercontent.com/2488155/152438720-d848fc75-b521-4e94-be3e-9c0f3e0dbb45.png)
When we get an error or no user actions:
![image](https://user-images.githubusercontent.com/2488155/152438757-33d0a297-a955-4c9e-9a86-32994ca58702.png)