LBRYCommunity/lbry-desktop

Fork 0

Enhance file remove modal #895

New issue

Closed

opened 2017-12-23 00:04:36 +01:00 by tzarebczan · 13 comments

tzarebczan commented

2017-12-23 00:04:36 +01:00

(Migrated from github.com)

The Issue

The current file remove modal can be confusing to users:

"from LBRY?" should probably be closer to something like "from the app", especially on claims that you don't own.
show the file path/name somewhere
rewording of claim abandoning, check on by default. Warn if unchecking - the file is removed from their publishes area and the user can't remove the deposit from the claim page anymore (needs to be deleted via transaction list or redownloaded).

System Configuration

Anything Else

Screenshots

## The Issue The current file remove modal can be confusing to users: ![file remove](https://user-images.githubusercontent.com/8120721/34314317-5ffa04ae-e740-11e7-9ecd-55e4c5088a63.png) 1) "from LBRY?" should probably be closer to something like "from the app", especially on claims that you don't own. 2) show the file path/name somewhere 3) rewording of claim abandoning, check on by default. Warn if unchecking - the file is removed from their publishes area and the user can't remove the deposit from the claim page anymore (needs to be deleted via transaction list or redownloaded). ## System Configuration   ## Anything Else  ## Screenshots

brentmclark commented

2019-10-14 04:32:02 +02:00

(Migrated from github.com)

Can I grab this one?

kauffj commented

2019-10-14 21:34:41 +02:00

(Migrated from github.com)

That'd be awesome @brentmclark !

👍 1

brentmclark commented

2019-10-15 01:24:12 +02:00

(Migrated from github.com)

Thanks @kauffj! I could use a little help with the verbiage, though. I'm not particularly familiar with this product.

So far, I have this when checked:

and this when unchecked:

Thanks @kauffj! I could use a little help with the verbiage, though. I'm not particularly familiar with this product. So far, I have this when checked: ![state when checked](https://cl.ly/5a67454f9c41/Image%202019-10-14%20at%206.21.41%20PM.png) and this when unchecked: ![state when unchecked](https://cl.ly/8fbe0ab3017e/Image%202019-10-14%20at%206.22.53%20PM.png)

neb-b commented

2019-10-15 04:34:14 +02:00

(Migrated from github.com)

@brentmclark Those are looking great!

brentmclark commented

2019-10-15 16:11:08 +02:00

(Migrated from github.com)

Thanks, @seanyesmunt! I still have some work to do to get the strings into app-strings.json and the styles into the SCSS files. The app-strings implementation looks pretty straightforward, but I could use some help finding the optimal place for my styles.

For example, the red copy above. Can I add a --color-warning variable to the _vars.scss file or should this be treated as a one-off?

Similarly, for the form field descriptions that rest atop their respective form fields; I see you're using BEM. Does it make sense to extend the fieldset-section block with something like fieldset-section__description and fieldset-section__description--subtext?

Thanks, @seanyesmunt! I still have some work to do to get the strings into `app-strings.json` and the styles into the SCSS files. The `app-strings` implementation looks pretty straightforward, but I could use some help finding the optimal place for my styles. For example, the red copy above. Can I add a `--color-warning` variable to the `_vars.scss` file or should this be treated as a one-off? Similarly, for the form field descriptions that rest atop their respective form fields; I see you're using `BEM`. Does it make sense to extend the `fieldset-section` block with something like `fieldset-section__description` and `fieldset-section__description--subtext`?

neb-b commented

2019-10-15 16:28:50 +02:00

(Migrated from github.com)

@brentmclark The app-strings will be populated automatically so you don't need to worry about those.

You can just use .error-text for the red text.

For the additional text we should probably just use the helper prop, since we use that in other places. Maybe both the error and description text should be passed as helper? Not sure how that would look.

@brentmclark The `app-strings` will be populated automatically so you don't need to worry about those. You can just use `.error-text` for the red text. For the additional text we should probably just use the `helper` prop, since we use that in other places. Maybe both the error and description text should be passed as `helper`? Not sure how that would look.

brentmclark commented

2019-10-15 16:50:03 +02:00

(Migrated from github.com)

@seanyesmunt Thanks for the quick feedback; I'll give these a shot tonight.

🎉 1

brentmclark commented

2019-10-16 04:25:16 +02:00

(Migrated from github.com)

@seanyesmunt .error-text worked great; thanks!

The helper prop renders content below the checkbox instead of above the checkbox. To me, it doesn't read properly when rendered below the checkbox in this instance. Thoughts?

@seanyesmunt `.error-text` worked great; thanks! The `helper` prop renders content below the checkbox instead of above the checkbox. To me, it doesn't read properly when rendered below the checkbox in this instance. Thoughts? ![helper text rendered below checkbox](https://cl.ly/ef0ed756ff0c/Image%202019-10-15%20at%209.22.42%20PM.png)

brentmclark commented

2019-10-16 20:16:20 +02:00

(Migrated from github.com)

Oh my goodness I'm dense 😛. How do you feel about this approach instead?

Oh my goodness I'm dense 😛. How do you feel about this approach instead? ![without error](https://cl.ly/ce0dc261f08e/Image%202019-10-16%20at%201.14.37%20PM.png) ![with error](https://cl.ly/3507f9c5681d/Image%202019-10-16%20at%201.14.43%20PM.png)

neb-b commented

2019-10-16 20:17:58 +02:00

(Migrated from github.com)

That's looking great!

🎉 1

brentmclark commented

2019-10-16 20:29:35 +02:00

(Migrated from github.com)

Awesome. I'll cut the PR tonight. Thanks for your help!

kauffj commented

2019-10-16 20:38:53 +02:00

(Migrated from github.com)

A copy few suggestions:

URI and claim are both words we generally avoid forcing users to learn. Maybe "Abandon on blockchain"? It would also be neat if the amount returned could be shown, e.g. "Abandon on blockchain (receive 0.5 LBC back).
Remove the word "also" for the first checkbox

The vertical spacing is also a bit inconcistent, but that doesn't necessarily need to be fixed and can probably be left to @seanyesmunt

A copy few suggestions: - URI and claim are both words we generally avoid forcing users to learn. Maybe "Abandon on blockchain"? It would also be neat if the amount returned could be shown, e.g. "Abandon on blockchain (receive 0.5 LBC back). - Remove the word "also" for the first checkbox The vertical spacing is also a bit inconcistent, but that doesn't necessarily need to be fixed and can probably be left to @seanyesmunt

e4drcf commented

2020-05-18 20:46:23 +02:00

(Migrated from github.com)

may be it's time to close this issue?)
it's in the master already)

may be it's time to close this issue?) it's in the master already)

👍 1