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

Add support for QR code context in configuration #614

Merged
merged 3 commits into from
Dec 10, 2020
Merged

Conversation

yfre
Copy link
Contributor

@yfre yfre commented Dec 10, 2020

add support for context "QRCode" to binding configuration view.
it can be used for example for HomeKit pairing.

it looks like this
image

Signed-off-by: Eugen Freiter [email protected]

@yfre yfre requested a review from a team as a code owner December 10, 2020 10:16
@yfre yfre changed the title add support for QR code context in configuration [mainUI] add support for QR code context in configuration Dec 10, 2020
Eugen Freiter and others added 2 commits December 10, 2020 12:34
Signed-off-by: Eugen Freiter <[email protected]>
@ghys
Copy link
Member

ghys commented Dec 10, 2020

Interesting and clever use case for a config parameter!

Normally those are user-editable (and they are, in the code view) so you'd want to be careful, but I can see the clear benefit for the HomeKit users, and you did the implementation, so thanks!

I made 2 changes:

  • use the webpack code splitting trick (https://webpack.js.org/guides/lazy-loading/) to put the vue-qrcode component (and the qrcode library in its separate bundle so that it's only loaded when needed (qrcode is 29kb minified, it's not that much but it doesn't hurt either!).
  • there are some light linting rules in place, when you develop the UI make sure to use for instance the ESLint VSCode extension to see and fix those errors (it doesn't break the build but still):
    image

@yfre
Copy link
Contributor Author

yfre commented Dec 10, 2020

Interesting and clever use case for a config parameter!

Normally those are user-editable (and they are, in the code view) so you'd want to be careful, but I can see the clear benefit for the HomeKit users, and you did the implementation, so thanks!

I made 2 changes:

  • use the webpack code splitting trick (https://webpack.js.org/guides/lazy-loading/) to put the vue-qrcode component (and the qrcode library in its separate bundle so that it's only loaded when needed (qrcode is 29kb minified, it's not that much but it doesn't hurt either!).
  • there are some light linting rules in place, when you develop the UI make sure to use for instance the ESLint VSCode extension to see and fix those errors (it doesn't break the build but still):
    image

@ghys thank you for the review and feedback with explanation. i learned something. it was my first UI PR, im more the backend guy :)

@ghys
Copy link
Member

ghys commented Dec 10, 2020

No problem 👍
This should go into the CONTRIBUTING.md file...

These are the VSCode extensions that are most useful:

  • Vetur (so useful it's almost required!)
  • ESLint (probably required too)
  • language-stylus (if you work on stylus)
  • Nearley (if you work on the nearley grammars)

@ghys ghys merged commit 3e95c34 into openhab:master Dec 10, 2020
@ghys ghys added this to the 3.0.0.RC1 milestone Dec 12, 2020
@ghys ghys added the enhancement New feature or request label Dec 12, 2020
@ghys ghys changed the title [mainUI] add support for QR code context in configuration Add support for QR code context in configuration Dec 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants