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

Review docs #56

Open
stropitek opened this issue Oct 20, 2023 · 2 comments
Open

Review docs #56

stropitek opened this issue Oct 20, 2023 · 2 comments
Assignees

Comments

@stropitek
Copy link
Contributor

If you could give your opinion on a few things that would be appreciated:

  • The structure of the menus and navbar
  • The definitions in the glossary
  • The structure of pages in "Features". Each page is supposed to use a similar structure.

https://image-js-docs.pages.dev/

@opatiny
Copy link

opatiny commented Oct 20, 2023

Okay so here's my feedback

Menus and navbar

  • in the top menu, you should remove the default "Blog" item, except if it's something that you actually want to keep
  • I like the glossary menu, I think it's useful
  • the concept menu is a bit confusing, I feel like I wouldn't really know what to search for in that menu. Maybe you could rename it into "Useful tips" or something like that? I would then separate the "Tips" and the "Tutorials" into two tabs at the main level
  • are you going to make a clear distinction between images and masks? In any case an explanation of the difference would be useful. I've seen there is one in the glossary, but maybe it would deserve a whole section "Images and Masks" or "Images VS Masks"?
  • they are still many features missing in the doc: ROIs, feature matching, etc. Do you intend to make separate menus for those inside of features? You should think well about the structure you'll adopt inside of "Features" so that it stays clear. (it will probably be easier to discuss about that)

Glossary definitions

I'l make a PR to fix minor issues.

  • the distinction between channel and component is not super clear
  • "Hysteresis" feels out of place. It sounds too specific compared to the other definitions. Either you'll add some more definitions and have everything at the same place of divide the glossary into more sub-sections like "Basic concepts", "Edge detection", "Feature matching", ... It is also not clear whereas it is generic image processing theory or concepts specific to image-js (for example with channels and components.

Structure of the "Features" page

  • I would add an explanation page saying what each of the little buttons does in the components that allow to test the feature on an image. And/or add some info when hovering on the buttons
  • The "Implementation" section at the bottom of the page is interesting, but isn't super visible since its hidden by default, and it seems to be a lot of work to write down how all of the features are implemented. I don't know if it's worth it. The people that actually need to know precisely how the function is implemented might just clone the project and go see for themselves.
  • Does the caution "This method only works with images." mean it doesn't work on masks? I don't find it super clear.
  • The main text in the features is quite long. I would add a one-sentence explanation just below the title.
    For example grayscale: Convert an RGB or RGBA image to gray.
  • We might add a section specifying the type of images accepted (aka the images allowed by checkProcessable). It would indicate the accepted color models and bit depths.
  • I wish the types were specified more clearly for each of the parameters and options.

@stropitek
Copy link
Contributor Author

Thanks a lot for the feedback! We created some issues out of most of the points.

  • The "Implementation" section at the bottom of the page is interesting, but isn't super visible since its hidden by default, and it seems to be a lot of work to write down how all of the features are implemented. I don't know if it's worth it. The people that actually need to know precisely how the function is implemented might just clone the project and go see for themselves.

I think we are going to keep it that way. The motivation behind it was:

  • Hidden because most people won't go that deep
  • It's there also so that @EscapedGibbon can read code / learn from it
  • If people interested in the source code had a way to go to the relevant part of the source code easily that would be interesting. The API documentation points to the source code but currently it's tricking to actually get to where the method is implemented because how image methods are just proxies for the actual implementation function.

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

No branches or pull requests

2 participants