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

feature: added tooltip on icons #6

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

taynambenichio
Copy link

Added tooltip on chat icons:

@taynambenichio taynambenichio deleted the feature/icon-with-tooltip branch August 19, 2024 23:58
@taynambenichio taynambenichio restored the feature/icon-with-tooltip branch August 19, 2024 23:58
@taynambenichio taynambenichio force-pushed the feature/icon-with-tooltip branch from 91841d3 to 983bdc6 Compare August 20, 2024 00:52
@taynambenichio taynambenichio changed the title Add feature: Added tooltip on icons feature: added tooltip on icons Aug 20, 2024
@Daniel-Boll Daniel-Boll changed the base branch from main to develop August 20, 2024 16:02
Copy link
Contributor

@DanielHe4rt DanielHe4rt left a comment

Choose a reason for hiding this comment

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

Your PR looks good but it has a small problem: it was started from main branch and not develop. This is our fault and thankfully not imply in your pull request.

Besides that: the new implementation from Develop has the SettingsOption which should be followed. Fixing that it will be ready to merge.

BTW: thanks for the contribution <3

@@ -61,8 +61,17 @@ const buildBadge = (occupation) => {
// SevenTV Stuff
badgeContainer.setAttributeNode(document.createAttribute("data-v-9f956e7d"));

// Get the occupation object
const occupationObject = occupations.find((o) => o.apiValue === occupation);
Copy link
Contributor

Choose a reason for hiding this comment

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

We changed branches and now the occupation variable passed in the scope returns the SettingsOption object:

export type SettingsOption = {
  name: string;
  slug: string;
  translation_key: string;
};

Comment on lines +70 to +74
const title =
occupationObject.translationKey === "None"
? "Twitch Better Profile"
: t(`occupation${occupationObject.translationKey}`);

Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

Suggested change
const title =
occupationObject.translationKey === "None"
? "Twitch Better Profile"
: t(`occupation${occupationObject.translationKey}`);
const title =
occupation.slug.toLowerCase() === "none"
? "Twitch Better Profile"
: t(`occupation${occupation.translation_key}`);

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.

2 participants