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

feat: add RandomTips #23

Merged
merged 19 commits into from
May 13, 2021
Merged

feat: add RandomTips #23

merged 19 commits into from
May 13, 2021

Conversation

rugk
Copy link
Owner

@rugk rugk commented Apr 18, 2021

No description provided.

@rugk rugk added this to the 0.5-beta milestone Apr 18, 2021
@rugk rugk marked this pull request as ready for review April 18, 2021 20:31
@rugk rugk requested a review from tdulcet April 18, 2021 20:35
@rugk
Copy link
Owner Author

rugk commented Apr 18, 2021

Hi @tdulcet Ito create the real links I’ve published v⅒ (edit: lol… this add-on is sometimes too funny 😆, v0.1) with the current main branch as a beta version: https://addons.mozilla.org/firefox/addon/unicodify-text-transformer/

Could you give me a mail from an AMO account of your’s (best mail it to me, see my mail in my profile) so I can add a developer there, so you can upload releases and such stuff?

tdulcet
tdulcet previously approved these changes Apr 19, 2021
Copy link
Collaborator

@tdulcet tdulcet left a comment

Choose a reason for hiding this comment

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

Thanks for doing this. I guess I should have done most of it in #2.

src/common/modules/data/Tips.js Outdated Show resolved Hide resolved
src/common/modules/data/Tips.js Show resolved Hide resolved
src/_locales/en/messages.json Outdated Show resolved Hide resolved
@tdulcet
Copy link
Collaborator

tdulcet commented Apr 19, 2021

Could you give me a mail from an AMO account of your’s (best mail it to me, see my mail in my profile) so I can add a developer there, so you can upload releases and such stuff?

No problem. I just sent you an e-mail.

@rugk rugk requested a review from tdulcet April 22, 2021 18:37
tdulcet
tdulcet previously approved these changes Apr 23, 2021
src/_locales/en/messages.json Outdated Show resolved Hide resolved
src/common/modules/LanguageHelper.js Show resolved Hide resolved
@tdulcet
Copy link
Collaborator

tdulcet commented Apr 23, 2021

I tried to test this branch and it seems the RandomTips library is not working:
image
It looks like it was broken by TinyWebEx/RandomTips@4468f27.

@rugk
Copy link
Owner Author

rugk commented Apr 23, 2021

Uf… okay, thanks. I’ve opened TinyWebEx/RandomTips#8. Need to look into this…

Also a very old commit, but it can be it never went into a release.
And yeah, that's bad/happens if one does not do/write (proper) tests, yeah…

@rugk
Copy link
Owner Author

rugk commented May 13, 2021

Okay, so just reverted that new feature of RandomTips, it caused problems anyway. See TinyWebEx/RandomTips#8

@rugk rugk requested a review from tdulcet May 13, 2021 12:55
src/options/options.js Outdated Show resolved Hide resolved
@rugk rugk requested a review from tdulcet May 13, 2021 13:39
tdulcet
tdulcet previously approved these changes May 13, 2021
Copy link
Collaborator

@tdulcet tdulcet left a comment

Choose a reason for hiding this comment

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

Note the unresolved comment above, which produces a couple errors:
image
This could be fixed in a follow up PR if needed.

@rugk
Copy link
Owner Author

rugk commented May 13, 2021

Sorry for fixing some other off-topic things, too. It should now work in the options page, too. 🙂

@rugk rugk requested a review from tdulcet May 13, 2021 14:30
Copy link
Collaborator

@tdulcet tdulcet left a comment

Choose a reason for hiding this comment

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

Looks good!

@rugk rugk merged commit 3138d3a into main May 13, 2021
@rugk rugk deleted the randomTips branch May 13, 2021 15:12
This was referenced May 22, 2021
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.

None yet

2 participants