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

Copy decoded qr #1491

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
Open

Copy decoded qr #1491

wants to merge 13 commits into from

Conversation

ataata107
Copy link

@ataata107 ataata107 commented Jan 14, 2020

Fixes #1471 (<=== Replace 0000 with the Issue Number)

Make sure these boxes are checked before your pull request (PR) is ready to be reviewed and merged. Thanks!
ezgif com-video-to-gif (8)

  • tests pass -- look for a green checkbox ✔️ a few minutes after opening your PR -- or run tests locally with npm test
  • code is in uniquely-named feature branch and has no merge conflicts
  • PR is descriptively titled
  • ask @publiclab/is-reviewers for help, in a comment below
  • Insert-step functionality is working correct as expected.

We're happy to help you get this ready -- don't be afraid to ask for help, and don't be discouraged if your tests fail at first!

If tests do fail, click on the red X to learn why by reading the logs.

Please be sure you've reviewed our contribution guidelines at https://publiclab.org/contributing-to-public-lab-software
Please make sure to get at least two reviews before asking for merging the PR as that would make the PR more reliable on our part
Thanks!

@ataata107 ataata107 changed the title Copy Copy decoded qr Jan 14, 2020
@ataata107
Copy link
Author

@publiclab/is-reviewers

@codecov
Copy link

codecov bot commented Jan 14, 2020

Codecov Report

Merging #1491 into main will increase coverage by 9.74%.
The diff coverage is 61.61%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1491      +/-   ##
==========================================
+ Coverage   55.11%   64.86%   +9.74%     
==========================================
  Files         117      133      +16     
  Lines        2344     2769     +425     
  Branches      360      442      +82     
==========================================
+ Hits         1292     1796     +504     
+ Misses       1052      973      -79     
Impacted Files Coverage Δ
examples/lib/scopeQuery.js 18.51% <ø> (ø)
src/Modules.js 100.00% <ø> (ø)
src/modules/WebglDistort/Module.js 2.29% <0.00%> (ø)
src/ui/SetInputStep.js 12.90% <0.00%> (-1.39%) ⬇️
src/modules/ColorHalftone/Module.js 3.63% <3.63%> (ø)
examples/lib/defaultHtmlStepUi.js 10.42% <3.84%> (-1.77%) ⬇️
examples/lib/intermediateHtmlStepUi.js 11.11% <5.55%> (+0.94%) ⬆️
examples/lib/insertPreview.js 13.15% <20.00%> (-0.36%) ⬇️
src/util/getImageDimensions.js 20.00% <20.00%> (ø)
src/util/isGif.js 20.00% <20.00%> (ø)
... and 102 more

Copy link
Member

@harshkhandeparkar harshkhandeparkar left a comment

Choose a reason for hiding this comment

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

I have requested some changes. Sorry if my initial issue was unclear.

examples/lib/defaultHtmlStepUi.js Outdated Show resolved Hide resolved
examples/lib/defaultHtmlStepUi.js Outdated Show resolved Hide resolved
examples/lib/mapHtmltypes.js Outdated Show resolved Hide resolved
src/modules/DecodeQr/info.json Outdated Show resolved Hide resolved
Copy link
Member

@harshkhandeparkar harshkhandeparkar left a comment

Choose a reason for hiding this comment

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

Please make the output field, a span or p instead of input or textarea so that the whole string is visible on the screen.

@ataata107
Copy link
Author

@harshkhandeparkar sorry for being late.
Done

@rishabhshuklax
Copy link
Member

Instead of doing this, we can do it in the Module.js itself like we do in Crop or Overlay, that would be much cleaner!

function modifiedCallback() {
if (options.step.inBrowser && !options.noUI) {
ui.setup();
}
callback();
}

@ataata107
Copy link
Author

There can be other modules which can have string output. Hence I only did it in defaultHtmlStepUi.js instead of changing module.js for all such cases

@gitpod-io
Copy link

gitpod-io bot commented Jul 8, 2020

Copy link
Member

@harshkhandeparkar harshkhandeparkar left a comment

Choose a reason for hiding this comment

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

Awesome!

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.

Better UI Output Fields
3 participants