Update modal for edits to existing claims (different verbiage) #2573

2019-06-25T06:24:26+02:00

jonnylynchy commented

2019-06-25 06:24:26 +02:00

(Migrated from github.com)

PR Checklist

Please check all that apply to this PR using "x":

I have checked that this PR is not a duplicate of an existing PR (open, closed or merged)
I have checked that this PR does not introduce a breaking change
This PR introduces breaking changes and I have provided a detailed explanation below

PR Type

What kind of change does this PR introduce?

Bugfix
Feature
Code style update (formatting)
Refactoring (no functional changes)
Documentation changes
Other - Please describe: improvement

Fixes

Issue Number:
https://github.com/lbryio/lbry-desktop/issues/2228

What is the current behavior?

When a user edits an existing claim and clicks "edit", the success modal has the original verbiage for a new publish.

What is the new behavior?

The modal that appears now says the updates have been published.

Other information

I wasn't sure if this is the best practice or not. The original publish success modal could receive props for modal title and content and change based on new or edit, but that would require some semi-significant changes upstream. Instead, I simply created a new modal with verbiage that reflects an update vs. a new publish.

## PR Checklist  Please check all that apply to this PR using "x": - [x] I have checked that this PR is not a duplicate of an existing PR (open, closed or merged) - [x] I have checked that this PR does not introduce a breaking change - [ ] This PR introduces breaking changes and I have provided a detailed explanation below ## PR Type What kind of change does this PR introduce? - [ ] Bugfix - [ ] Feature - [ ] Code style update (formatting) - [ ] Refactoring (no functional changes) - [ ] Documentation changes - [x] Other - Please describe: _improvement_ ## Fixes Issue Number: https://github.com/lbryio/lbry-desktop/issues/2228 ## What is the current behavior? When a user edits an existing claim and clicks "edit", the success modal has the original verbiage for a new publish. ![image](https://user-images.githubusercontent.com/1364707/60068764-baeff800-96d5-11e9-94e6-79623687eeec.png) ## What is the new behavior? The modal that appears now says the **updates** have been published. ![image](https://user-images.githubusercontent.com/1364707/60069081-1f5f8700-96d7-11e9-8883-c7ce792d2902.png) ## Other information I wasn't sure if this is the best practice or not. The original publish success modal could receive props for modal title and content and change based on new or edit, but that would require some semi-significant changes upstream. Instead, I simply created a new modal with verbiage that reflects an update vs. a new publish.

jonnylynchy (Migrated from github.com) reviewed 2019-06-25 15:34:34 +02:00

static/locales/en.json

					
				@ -289,0 +287,4 @@

				  "% complete": "% complete",

				  "Updates published": "Updates published",

				  "Your updates have been published to LBRY at the address": "Your updates have been published to LBRY at the address",

				  "The updates will take a few minutes to appear for other LBRY users. Until then your file will be listed as \"pending\" under your published files.": "The updates will take a few minutes to appear for other LBRY users. Until then your file will be listed as \"pending\" under your published files."

jonnylynchy (Migrated from github.com) commented

2019-06-25 15:34:33 +02:00

I see comma-dangle in .eslintrc.json and .prettierrc.json, but doesn't look like it's being respected.

I see `comma-dangle` in _.eslintrc.json_ and _.prettierrc.json_, but doesn't look like it's being respected.

neb-b (Migrated from github.com) requested changes 2019-06-25 21:42:46 +02:00

neb-b (Migrated from github.com) left a comment

One comment

src/ui/redux/actions/publish.js

neb-b (Migrated from github.com) commented

2019-06-25 21:42:29 +02:00

Instead of creating a new publish modal, we could just pass in a value here

    actions.push(doOpenModal(MODALS.PUBLISH, { uri, isEdit }));

Then change the text based on that prop.

Instead of creating a new publish modal, we could just pass in a value here ``` actions.push(doOpenModal(MODALS.PUBLISH, { uri, isEdit })); ``` Then change the text based on that prop.

neb-b commented

2019-06-25 21:43:58 +02:00

(Migrated from github.com)

Thank you for your first PR! I just had one comment that should simplify this quite a bit.

Can we show you some appreciation for your contribution?
https://lbry.com/faq/appreciation

Thank you for your first PR! I just had one comment that should simplify this quite a bit. Can we show you some appreciation for your contribution? https://lbry.com/faq/appreciation

jonnylynchy (Migrated from github.com) reviewed 2019-06-26 00:34:15 +02:00

src/ui/redux/actions/publish.js

jonnylynchy (Migrated from github.com) commented

2019-06-26 00:34:15 +02:00

Yup, much better solution. I'll change. Thanks!

jonnylynchy commented

2019-06-26 04:33:28 +02:00

(Migrated from github.com)

Absolutely Sean. Thanks!

On Tue, Jun 25, 2019 at 2:44 PM Sean Yesmunt notifications@github.com
wrote:

Thank you for your first PR! I just had one comment that should simplify
this quite a bit.

Can we show you some appreciation for your contribution?
https://lbry.com/faq/appreciation

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/lbryio/lbry-desktop/pull/2573?email_source=notifications&email_token=AAKNFY3GQ6S6YPPMO7ZH763P4JYQHA5CNFSM4H3EIWZ2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODYRMBQQ#issuecomment-505594050,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAKNFY5TACYZ3QIK6AFIFGLP4JYQHANCNFSM4H3EIWZQ
.

Absolutely Sean. Thanks! On Tue, Jun 25, 2019 at 2:44 PM Sean Yesmunt <notifications@github.com> wrote: > Thank you for your first PR! I just had one comment that should simplify > this quite a bit. > > Can we show you some appreciation for your contribution? > https://lbry.com/faq/appreciation > > — > You are receiving this because you authored the thread. > Reply to this email directly, view it on GitHub > <https://github.com/lbryio/lbry-desktop/pull/2573?email_source=notifications&email_token=AAKNFY3GQ6S6YPPMO7ZH763P4JYQHA5CNFSM4H3EIWZ2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODYRMBQQ#issuecomment-505594050>, > or mute the thread > <https://github.com/notifications/unsubscribe-auth/AAKNFY5TACYZ3QIK6AFIFGLP4JYQHANCNFSM4H3EIWZQ> > . >

jonnylynchy commented

2019-06-26 20:20:21 +02:00

(Migrated from github.com)

🕺

Sign in to join this conversation.

No reviewers

neb-b