Skip to content
This repository has been archived by the owner on Jul 19, 2022. It is now read-only.

New Web Manifest creating / auditing experience #68

Merged
merged 31 commits into from
May 4, 2022
Merged

New Web Manifest creating / auditing experience #68

merged 31 commits into from
May 4, 2022

Conversation

jgw96
Copy link
Contributor

@jgw96 jgw96 commented Apr 25, 2022

This PR implements the following:

  • Instead of a form we have moved to a full-featured VSCode snippet to guide the user in editing their Web Manifest, complete with tooltips, inline docs and multi selects for values where there are default values
  • Every member of the Web Manifest now has a hover tooltip with in-line docs + a link to learn more if needed. However, the in-line info should cover most of what the dev will need to know.
  • New diagnostics (aka validations) for the Web Manifest that cover:
    • Whitespaces
    • incorrect values
    • values of the wrong type
    • incorrect JSON
    • recommended values
    • required values
  • Quick Fixes for each diagnostic
    • Each diagnostic has a quick fix, enabling the dev to fix the problem by just clicking a button in VSCode, as they have most likely already done in VSCode before.

With the above this PR completes:

@nmetulev
Copy link
Member

Awesome work. Few questions/comments:

  • I'm expecting to have autocomplete when adding new properties so I know what properties I can add, similar to how editing a package.json works. Is that supposed to work yet? It is working for values of a property, but not for adding the properties themselves
  • is it possible to add an inline command above the icons property that I can click to generate the icons (similar to how I can start debugging directly from package.json)?
  • is it possible to add a color picker for colors, similar to how a color picker works for css?
  • The extension thinks I have a manifest even after I've deleted my manifest, is this a known issue?

@jgw96
Copy link
Contributor Author

jgw96 commented Apr 27, 2022

Issues for things id like to add from the review so far. Note: These will be implemented separate from this PR. This is more just for my organization of things.

@zateutsch
Copy link
Collaborator

I like this editing experience a lot. One thing to note though, if you generate a new manifest, it doesn't have the screenshots field, and it results in warning highlight throughout the manifest. Could we just highlight the final bracket or something similar?

image

This makes the other warnings/info hard to read, also pretty visually distracting.

@zateutsch
Copy link
Collaborator

Also, extension is doing manifest validation in non-manifest json files.

image

@jgw96 jgw96 marked this pull request as draft April 29, 2022 17:55
@jgw96 jgw96 marked this pull request as ready for review April 29, 2022 22:45
@jgw96
Copy link
Contributor Author

jgw96 commented Apr 29, 2022

@nmetulev and @zateutsch this PR is ready for another review. I fixed the bugs that both of yall pointed out and made some UX tweaks based on Zach's feedback. Nikola, your two feature requests have been created too, and will be implemented separately from this PR.

@jgw96 jgw96 merged commit 652c172 into main May 4, 2022
@jgw96 jgw96 deleted the mani-snip branch May 4, 2022 16:28
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.