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

Added screen recordings of functions and took screenshots #284

Closed
wants to merge 2 commits into from

Conversation

Tch4lla
Copy link
Contributor

@Tch4lla Tch4lla commented Sep 19, 2022

I hope these are what you were looking for. I kept the file names the same in case they were referenced somewhere in the code base

@Tch4lla
Copy link
Contributor Author

Tch4lla commented Oct 3, 2022

Reaching out again as it has been a fortnight without any conformation about the status of this pull request.

@rugk
Copy link
Owner

rugk commented Oct 19, 2022

Hi @Tch4lla,
thanks for your first contribution to this project! 🎉 👍 🏅
I hope you'll like this project and enjoy hacking on it… 😃

Sorry for the delay, I missed the notifications.

You can (automatically) let issues close when a PR is merged by adding some "magic" text to your PR body. (Manually linked it now.)

BTW: Next time, try to avoid doing a pull request from the master branch, because you can run into problems when you have a "non-clean" master that does not follow this repo here (i.e. "upstream"). See this article for details. Anyway, this is only a tip for the next time. 😃

Copy link
Owner

@rugk rugk left a comment

Choose a reason for hiding this comment

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

Hi and thanks again, these look very good.

Some notes, though:

  • Note that your qrText.gif accidentially I guess (or hope) shows an kinda "incorrect" usage, unfortunately: The add-on as shown automatically selects the whole text for you, but you anyway clicked somewhere and double- (or triple) clicked to select the whole text.
    That was just an unnecessary step and to show that in an official demo is likely not that optimal. (So people know it is easier than what you showed there.)
  • qrBig.png shows example.org, which is okay, but I used the GitHub URL before as that as again some useful ressource for visitors and a longer URL, where a big QR code makes sense. So again I would prefer to keep that bigger one.
  • qrSettings.png should show the settings of the add-on. As such, please switch to "options" and create a screenshot of the whole page (not the add-on description). It may be needed you access the add-on via Firefox extension devtools and take a screenshot there](https://firefox-source-docs.mozilla.org/devtools-user/taking_screenshots/index.html) if the options page is too long for your screen.

So if you wanted to correct that, that would be great.

@rugk rugk linked an issue Oct 19, 2022 that may be closed by this pull request
@rugk
Copy link
Owner

rugk commented Oct 19, 2022

Also note BTW if #286 is merged (you can try out the test branch already), we can demonstrate another feature as a GIF, i.e. copy text into clipboard and then opening the add-on popup.
That is not really needed for this here/yet, but I just wanted to say that.

@Tch4lla
Copy link
Contributor Author

Tch4lla commented Oct 30, 2022

Ahh I should have commented here. I have made a separate pull request, so I will close this one since the other has all of the changes in the way that you have asked, including the new branch

@Tch4lla Tch4lla closed this Oct 30, 2022
@rugk
Copy link
Owner

rugk commented Dec 6, 2022

No need for a separate PR, but thanks.

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.

Recreate screenhots and screencasts in new Firefox design
2 participants