Skip to content

Fixes #26 - Issue reporter screenshot feature

Andrea Giammarchi requested to merge issue-reporter into release-2018-3

There are various questions I have around these changes, and while the code is currently not ready for production, the PR is going to be big + I am blocked by https://codereview.adblockplus.org/29785555/ that doesn't have a LGTM so that I cannot test anything I am doing, which is a bummer.

However, the main logic to put components together and move on while reporting are here, and ready to be reviewed where it makes sense.

What is missing, or what I would like to understand, is the following:

  • why I don't see bottom right buttons content even from master ?
  • are all extension tabs related operation meant to be only at the end or I can Promise.all all operation and release the issue reporter once these have been collected?
  • what happened to the max length message warning that is nowhere in specs? I've removed it but should we put a counter or any info about such max amount of chars and use maxlength on the textarea instead ?
  • do I need to do anything different from creating that XML to send the report now that we have the screenshot ? I don't remember if I should just add the base64 data or there is something else

There are more questions probably to follow, but so far I'd love an initial review of the current status which is everything but the logic that was in there before.

Last, but not least, the space for the screenshot, even if shown horizontal, is not much ... I think we need to define a smaller font size in general, so there would be more space, but I also think there should be a minimum canvas height to show the collected screenshot otherwise users might not realize there is even their data exposed in it.

To test locally, after cloning/cherry-picking, simply run npm start and open issue-reporter.html in the localhost:8080.

Edited by Andrea Giammarchi

Merge request reports