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 screenshots to npm readmes (?) #1245

Closed
wwalc opened this issue Sep 14, 2018 · 13 comments
Closed

Add screenshots to npm readmes (?) #1245

wwalc opened this issue Sep 14, 2018 · 13 comments
Assignees
Labels
type:task This issue reports a chore (non-production change) and other types of "todos".
Milestone

Comments

@wwalc
Copy link
Member

wwalc commented Sep 14, 2018

There is a chance that with screenshots npm readmes would look even better:

@wwalc wwalc changed the title Add screnshots to npm readmes (?) Add screenshots to npm readmes (?) Sep 14, 2018
@Reinmar Reinmar added status:confirmed type:task This issue reports a chore (non-production change) and other types of "todos". labels Sep 14, 2018
@Reinmar Reinmar added this to the iteration 20 milestone Sep 14, 2018
@Reinmar
Copy link
Member

Reinmar commented Sep 14, 2018

One thing to research first – where to host those images. Can they be stored in repositories or does GitHub disallow hotlinking?

@Reinmar
Copy link
Member

Reinmar commented Sep 14, 2018

The readme we've got on https://github.com/ckeditor/ckeditor5 is hosted on our CDN so maybe we'll need to go this way.

@wwalc
Copy link
Member Author

wwalc commented Sep 14, 2018

CKEditor 4 holds the screenshot in a dedicated assets folder: https://github.com/ckeditor/ckeditor-dev/tree/master/.npm/assets and https://www.npmjs.com/package/ckeditor links to https://raw.githubusercontent.com/ckeditor/ckeditor-releases/HEAD/assets/ckeditor4.png
However the path should be fixed instead of pointing to HEAD.

CC @mlewand as he may have some further comments on their approach.

I don't have any personal preferences.

@dkonopka
Copy link
Contributor

I've reduced height of images by ~30% to save some space in the readmes. WDYT?

Current https://github.com/ckeditor/ckeditor5 image

68747470733a2f2f632e636b736f757263652e636f6d2f612f312f696d672f6e706d2f636b656469746f7225323035253230636c6173736963253230736372656573686f742e706e67 1

Balloon proposal:

balloon-proposal

Classic proposal:

classic-proposal

Inline proposal:

inline-proposal

PS: Maybe we should think about improving features readmes too?

Highlight readme proposal:

highlight-proposal2

Font family:

font-family-proposal

@Reinmar
Copy link
Member

Reinmar commented Sep 17, 2018

Font family:

👍

I've reduced height of images by ~30% to save some space in the readmes. WDYT?

I'm not sure. E.g. we make image caption invisible in the classic editor case. I like the current height more.

PS: Maybe we should think about improving features readmes too?

We might... but I'd rather link to a demo instead (which I think we don't do). It says more than this and means less work for us. BTW, we should also check docs/api/<feature name>.md files for the same.

@Reinmar
Copy link
Member

Reinmar commented Sep 17, 2018

Damian made me think about the docs/api/<feature name>.md pages. I see e.g. that in autoformat we link to the feature's page (where there's a demo) but we don't use the word "demo". I'd add another section before ### Documentation with a simple:

### Demo

Check out the demo in the {@link features/autoformat Autoformat feature} guide.

### Documentation

See the {@link features/autoformat Autoformat feature} guide and the {@link module:autoformat/autoformat~Autoformat} plugin documentation.

WDYT? cc @AnnaTomanek

@AnnaTomanek
Copy link
Contributor

Demo links are nice, but images are something far more eye-catching. So I'm afraid I'm +1 about adding screenshots, too :)

See also #1186, #1187.

@Reinmar
Copy link
Member

Reinmar commented Sep 17, 2018 via email

@dkonopka
Copy link
Contributor

(...) I'd rather link to a demo instead (which I think we don't do). It says more than this (...)

I don't agree with this, clicking the demo url is an additional action for user and a lot of them doesn't use/see it. Of course is time-consuming for us, but still, we should think about it.

Anyway, take a look at fixed screenshots, if you are ok with them I'll prepare a PR.

Balloon build

balloonv2

Classic build

classicv2

Inline build

inlinev2

Decoupled build

decoupled

@Reinmar
Copy link
Member

Reinmar commented Sep 17, 2018

I don't agree with this, clicking the demo url is an additional action for user and a lot of them doesn't use/see it. Of course is time-consuming for us, but still, we should think about it.

And so we are thinking :D What we need to focus on is the ROI. It's clear that screenshots are great and say more than 1000 words. Still:

Taken the above into account, I say – no screenshots, please. Unless we can automate making them (possible, but future), that's a big cost and a low ROI.

However, we still have a problem – you can't quickly get to a demo from https://github.com/ckeditor/ckeditor5-alignment. There's no link to either this feature's documentation or, better, the demo itself. And that' something we can improve.

That's why on GitHub, npm and in API docs I'd use such a text:

### Demo

Check out the demo in the {@link features/autoformat Autoformat feature} guide.

### Documentation

See the {@link features/autoformat Autoformat feature} guide and the [`@ckeditor/ckeditor5-alignment` package](https://ckeditor.com/docs/ckeditor5/latest/api/alignment.html) page in [CKEditor 5 documentation](https://ckeditor.com/docs/ckeditor5/latest/).

(with slight adjustments in certain cases)

Thanks to how clear the readme is, a person who's checking out this feature will have no problems with finding the demo.

@Reinmar
Copy link
Member

Reinmar commented Sep 17, 2018

Anyway, take a look at fixed screenshots, if you are ok with them I'll prepare a PR.

👍

@wwalc
Copy link
Member Author

wwalc commented Sep 18, 2018

Just $0.02 about screenshots - I personally don't like screenshots with selection, unless it's really needed for showcasing something. It's a little bit confusing with the highlight feature. I personally prefer showing just the collapsed selection somewhere (at the end of header).

No need to rework screenshots, just checking if others share my preferences (for the future or sth) or if I'm alone :D

@dkonopka
Copy link
Contributor

dkonopka commented Sep 18, 2018

Hmm.. I did it intuitively 😄plus don't have any preferences which version is better ;)

Anyways, I just realised that screenshots need an update because of missing table & media-embed(?) features.

Reinmar added a commit to ckeditor/ckeditor5-build-decoupled-document that referenced this issue Sep 21, 2018
Internal: Added build screenshot to README.md. Closes ckeditor/ckeditor5#1245.
Reinmar added a commit to ckeditor/ckeditor5-build-inline that referenced this issue Sep 21, 2018
Internal: Added build screenshot to README.md. Closes ckeditor/ckeditor5#1245.
Reinmar added a commit to ckeditor/ckeditor5-build-balloon that referenced this issue Sep 21, 2018
Internal: Added build screenshot to README.md. Closes ckeditor/ckeditor5#1245.
Reinmar added a commit to ckeditor/ckeditor5-build-classic that referenced this issue Sep 21, 2018
Internal: Added build screenshot to README.md. Closes ckeditor/ckeditor5#1245.
JDinABox pushed a commit to JDinABox/ckeditor5-build-markdown that referenced this issue Sep 6, 2021
Internal: Added build screenshot to README.md. Closes ckeditor/ckeditor5#1245.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:task This issue reports a chore (non-production change) and other types of "todos".
Projects
None yet
Development

No branches or pull requests

4 participants