qmk / qmk_configurator

The QMK Configurator
http://config.qmk.fm
669 stars 340 forks source link

Feature request: UI changes to reflect recent changes to the API #692

Closed skullydazed closed 4 years ago

skullydazed commented 4 years ago

The API recently changed so that the JSON is used directly to compile. This change has been refined and those refinements are currently available at a separate URL:

https://api.clueboard.co/v1 (API)

https://config.clueboard.co/ (Configurator)

Changes needed on the configurator side:

skullydazed commented 4 years ago

Here's an example JSON that includes the new keys (with the layers omitted for readability sake):

{
    "notes": "",
    "version": 1,
    "keyboard": "clueboard/66/rev4",
    "keymap": "mine",
    "layout": "LAYOUT",
    "layers": [...],
    "documentation": "This file is a QMK Configurator export. You can import this at <https://config.qmk.fm>. It can also be used directly with QMK's source code.

To setup your QMK environment check out the tutorial: https://docs.qmk.fm/#/newbs

You can convert this file to a keymap.c using this command: `qmk json2c {keymap}`

You can compile this keymap using this command: `qmk compile {keymap}`",
}
yanfali commented 4 years ago

Can you put an endpoint on the API with the documentation and version? that way the UI can just pull it at startup and always have the latest version. This also put's it's content under the API's control.

yanfali commented 4 years ago

The API recently changed so that the JSON is used directly to compile. This change has been refined and those refinements are currently available at a separate URL:

https://api.clueboard.co/v1 (API)

https://config.clueboard.co/ (Configurator)

Is this your test infrastructure?

Changes needed on the configurator side:

* Merge the Download Keymap and Export Keymap functions

Would removing the download keymap work?

* Find a way to point users to the newbs guide if they need more functionality

Sorry, what did you have in mind? You want a visual change?

* Make sure that the JSON sent to the API is exactly the same as what the user exports from configurator

I don't understand this comment. Can you give examples?

* Add two new keys to the export JSON- `version` and `documentation`

That seems straightforward.

skullydazed commented 4 years ago

Can you put an endpoint on the API with the documentation and version?

Sure. What if it returned a skeleton the UI could populate?

Is this your test infrastructure?

It's a separate API stack setup on the existing QMK infrastructure. I haven't had a chance to talk to @jackhumbert about setting up a new hostname, and we might want to talk as a group about what hostname to use.

Would removing the download keymap work?

Yeah, I think that's fine.

  • Find a way to point users to the newbs guide if they need more functionality

Sorry, what did you have in mind? You want a visual change?

I could think of several ways to accomplish what I wanted, so I didn't want to specify a specific solution. Essentially, I want users to have a way to discover there is more they can do if they use configurator to build a keymap and have a local source tree. Maybe this is accomplished with a help icon that opens a paragraph or two of text. Maybe it's a modal window that pops up with the wizard. Maybe it's something I haven't thought of. We can brainstorm a bit on discord if you want.

  • Make sure that the JSON sent to the API is exactly the same as what the user exports from configurator

I don't understand this comment. Can you give examples?

I don't know how things work under the hood, maybe this is already done. Since the API stores the JSON file inside the source download now I want to make sure that the JSON in the source download and the JSON from an export are exactly the same.

yanfali commented 4 years ago

Can you put an endpoint on the API with the documentation and version?

Sure. What if it returned a skeleton the UI could populate?

Is this your test infrastructure?

It's a separate API stack setup on the existing QMK infrastructure. I haven't had a chance to talk to @jackhumbert about setting up a new hostname, and we might want to talk as a group about what hostname to use.

Would removing the download keymap work?

Yeah, I think that's fine.

  • Find a way to point users to the newbs guide if they need more functionality

Sorry, what did you have in mind? You want a visual change?

I could think of several ways to accomplish what I wanted, so I didn't want to specify a specific solution. Essentially, I want users to have a way to discover there is more they can do if they use configurator to build a keymap and have a local source tree. Maybe this is accomplished with a help icon that opens a paragraph or two of text. Maybe it's a modal window that pops up with the wizard. Maybe it's something I haven't thought of. We can brainstorm a bit on discord if you want.

OK, I think I understand. perhaps we can send them to a docs page?

  • Make sure that the JSON sent to the API is exactly the same as what the user exports from configurator

I don't understand this comment. Can you give examples?

I don't know how things work under the hood, maybe this is already done. Since the API stores the JSON file inside the source download now I want to make sure that the JSON in the source download and the JSON from an export are exactly the same.

Ah I see what you're saying. There may be some corner cases involving ANY.

This may be tricky because of the ANY key, and because we handle keyboard renames automatically on import. Looking at how we handle ANY, we basically wrap those keycodes and treat them as text, ignoring them from a keycode point of view and passing them through. A common use case is the nested modifier case which the UI doesn't handle at this time.

It would be pretty easy to test this one use case. Right now the UI just re-wraps unknown codes with ANY I think on import so in theory this is handled seamlessly right now.

skullydazed commented 4 years ago

OK, I think I understand. perhaps we can send them to a docs page?

That seems reasonable.

Ah I see what you're saying. There may be some corner cases involving ANY.

Per our conversation on discord https://github.com/qmk/qmk_firmware/issues/8556 should bring the ANY() handling on the CLI in line with Configurator.

noroadsleft commented 4 years ago

UI stuff: Here, have a branch:

https://github.com/qmk/qmk_configurator/tree/issue-692

yanfali commented 4 years ago

Can you put an endpoint on the API with the documentation and version?

Sure. What if it returned a skeleton the UI could populate?

Yes this would work.

yanfali commented 4 years ago

Screen Shot 2020-04-04 at 19 23 52 @noroadsleft the CSS is a bit off on the new button

yanfali commented 4 years ago

Screen Shot 2020-04-04 at 19 23 52 @noroadsleft the CSS is a bit off on the new button

never mind. I think it was just a css issue.

noroadsleft commented 4 years ago

There should be an icon on the Help button:

image

yanfali commented 4 years ago

Fixed it. Let me check if I didn't committed

On Sat, Apr 4, 2020, 22:59 James Young notifications@github.com wrote:

There should be an icon on the Help button:

[image: image] https://user-images.githubusercontent.com/18669334/78468001-d9087100-76c7-11ea-8d06-47402dfe78e9.png

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/qmk/qmk_configurator/issues/692#issuecomment-609364078, or unsubscribe https://github.com/notifications/unsubscribe-auth/AARLSU22NPJVGA6RLPGMLKLRLAM3DANCNFSM4LTT5YMQ .

yanfali commented 4 years ago

@noroadsleft it's in there. it was missing briefly from main.js after merge

yanfali commented 4 years ago

@noroadsleft ok this may be ready to go. The API endpoint from the templates is hardcoded until it goes live when we'll switch to the same root.

yanfali commented 4 years ago

@skullydazed it's deployed. One caveat, when you deploy the new template endpoint to prod let us know and we'll switch the URL over to the correct end point. Right now it's using the experimental one.

yanfali commented 4 years ago

I think this is resolved now.