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

Make the media delete message scarier #15438

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
6 participants
@iamgabrielma
Copy link
Contributor

iamgabrielma commented Jun 23, 2017

Improves #10982 message by clarifying that deleted media items cannot be recovered.

Got some errors when pre-commiting:

`/Users/gma992/Dropbox/wp-calypso/client/my-sites/media/main.jsx
199:1 error Line 199 exceeds the maximum line length of 140 max-len
200:1 error Line 200 exceeds the maximum line length of 140 max-len

/Users/gma992/Dropbox/wp-calypso/client/post-editor/media-modal/index.jsx
196:1 error Line 196 exceeds the maximum line length of 140 max-len
197:1 error Line 197 exceeds the maximum line length of 140 max-len

/Users/gma992/Dropbox/wp-calypso/client/post-editor/media-modal/test/index.jsx
79:1 error Line 79 exceeds the maximum line length of 140 max-len
92:1 error Line 92 exceeds the maximum line length of 140 max-len
108:1 error Line 108 exceeds the maximum line length of 140 max-len
121:1 error Line 121 exceeds the maximum line length of 140 max-len`

But do not exceed 140 chars. any advice is welcome, as is my first PR :)

@matticbot matticbot added the [Size] S label Jun 23, 2017

@matticbot

This comment has been minimized.

Copy link

matticbot commented Jun 23, 2017

@iamgabrielma

This comment has been minimized.

Copy link
Contributor Author

iamgabrielma commented Jun 23, 2017

Example:
screen shot 2017-06-23 at 14 19 30

@matticbot

This comment has been minimized.

Copy link

matticbot commented Aug 16, 2017

@iamgabrielma This PR needs a rebase

@gwwar

This comment has been minimized.

Copy link
Member

gwwar commented Aug 30, 2017

@iamgabrielma you can break up the text using

translate( 'a very long string' +
     'other parts of the string' +
     'even more strings'
);

see also https://github.com/Automattic/wp-calypso/blob/master/docs/coding-guidelines/javascript.md#multi-line-statements

@michelleweber

This comment has been minimized.

Copy link
Member

michelleweber commented Aug 30, 2017

For the first line, I might replace "functioning" to make things a little simpler/clearer -- e.g., "Deleted media will no longer appear anywhere on your website."

If you want to go even stronger/clearer, "Deleted media will no longer appear anywhere on your website, including all posts, pages, and widgets"

@iamgabrielma

This comment has been minimized.

Copy link
Contributor Author

iamgabrielma commented Aug 30, 2017

This PR is a bit outdated, as new changes were merged here #10982 and now the message and prompt is scarier:

screen shot 2017-08-30 at 21 34 13

I'm happy to change for the current Are you sure you want to delete these items? They will be permanently removed from all other locations where they currently appear for Deleted media will no longer appear anywhere on your website, including all posts, pages, and widgets is that makes it clearer, or close the PR otherwise. Thoughts @michelleweber @gwwar ?

@gwwar

This comment has been minimized.

Copy link
Member

gwwar commented Aug 30, 2017

I think the new copy is clearer 👍

@michelleweber

This comment has been minimized.

Copy link
Member

michelleweber commented Aug 30, 2017

I think it's clearly to specify where things will no longer appear rather than the more general "all other locations." It's not unclear as-is, I just think the updated one is a little more clear :)

@iamgabrielma

This comment has been minimized.

Copy link
Contributor Author

iamgabrielma commented Aug 31, 2017

Closing this PR in favor of this one: #17675 with the current changes

Seems that when I was trying to rebase I messed something up and created a new PR instead :D

@gwwar

This comment has been minimized.

Copy link
Member

gwwar commented Aug 31, 2017

@iamgabrielma heads up that we don't necessarily need to close PRs if we mess up a rebase. As long as we didn't force push something funny to the branch, we can always try again locally.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment