Added necessary tooltips to icons in new frontend #2693 #2952
Conversation
Pulling New Updates
bryankhor07
changed the title
There was a problem hiding this comment.
First round review:
- Thanks for contributing and tackling the tooltip task.
- Please consider working from a separate branch in your fork and not from master.
- Please keep your PR clean and do not submit files that are not necessary or related to the fix you are working on.
- Please see the comments below.
There was a problem hiding this comment.
Please keep your PR clean and only submit files that are needed for your changes. The login.html file is not needed for the tooltip adjustments and should be removed from the PR please.
The Timesketch team builds a new UI on a regular basis to include all new changes.
There was a problem hiding this comment.
Please keep your PR clean and only include files needed for the issue you are fixing. The yarn.lock file is not needed for this PR. Please remove it from the PR.
| @@ -34,7 +34,7 @@ limitations under the License. | |||
| </v-list-item> | |||
| <v-list-item style="cursor: pointer" @click="showContextWindow()"> | |||
| <v-list-item-icon> | |||
| <v-icon small>mdi-magnify-plus-outline</v-icon> | |||
| <v-icon small>mdi-magnify-plus-outline</v-icon> <!--Add tooltip here? ()--> | |||
There was a problem hiding this comment.
No, this entry has an explanation next to it. It does not need a tooltip therefore.
| <!-- Add tooltip here? (Icons in the Threat Intelligence feature) --> | ||
| <span> <v-icon left>mdi-shield-search</v-icon> Threat Intelligence </span> |
| @@ -37,7 +38,7 @@ limitations under the License. | |||
| @click="addIndicator()" | |||
| @click.stop="" | |||
| > | |||
| <v-icon>mdi-plus</v-icon> | |||
| <v-icon title="Add Indicator">mdi-plus</v-icon> | |||
There was a problem hiding this comment.
This icon has the same functionality as the one above (line 32). It should have the same tooltip.
| @@ -92,7 +93,7 @@ limitations under the License. | |||
|
|
|||
| <template v-slot:item.actions="{ item }"> | |||
| <v-btn icon small @click="generateSearchQuery(item.ioc)"> | |||
| <v-icon small>mdi-magnify</v-icon> | |||
| <v-icon small title="Search">mdi-magnify</v-icon> | |||
There was a problem hiding this comment.
Can this title be a bit more descriptive? e.g. "Search this indicator" Same for the other mdi-magnify "search" icons in this section.
| @@ -33,7 +33,7 @@ limitations under the License. | |||
| <template v-slot:activator="{ on, attrs }"> | |||
| <v-avatar> | |||
| <v-btn small icon v-bind="attrs" v-on="on"> | |||
| <v-icon>mdi-dots-vertical</v-icon> | |||
| <v-icon title="Action Menu">mdi-dots-vertical</v-icon> | |||
There was a problem hiding this comment.
| <v-icon title="Action Menu">mdi-dots-vertical</v-icon> | |
| <v-icon title="Sketch menu">mdi-dots-vertical</v-icon> |
| @@ -63,7 +64,7 @@ limitations under the License. | |||
| ]" | |||
| > | |||
| <v-btn icon @click="toggleLeftPanel"> | |||
| <v-icon>mdi-menu</v-icon> | |||
| <v-icon title="Manage">mdi-menu</v-icon> | |||
There was a problem hiding this comment.
| <v-icon title="Manage">mdi-menu</v-icon> | |
| <v-icon title="Toggle left panel">mdi-menu</v-icon> |
|
Hi @jkppr , thanks for checking in! Yes, we are still revising the code based on the feedback. |
|
I forked this PR and finished it, since some UX guidance changed mid-way and we want to have it ready for the next release in December. Therefore closing this PR in favor of #2983 . Thanks for contributing @gstgrace @bryankhor07 @lemonshark05 |
It adds tooltips to icons that need a short description.
Checks
Closing issues
Closes #2693
Co-authors: @lemonshark05 @gstgrace