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

Share speech URLs on File and Channel pages #1222

Merged
merged 2 commits into from May 30, 2018

Conversation

daovist
Copy link
Contributor

@daovist daovist commented Mar 30, 2018

New PR for #1028 @tzarebczan @liamcardenas

This creates <ViewOnWebButton> with a GLOBE icon and uses it on <FilePage> and <ChannelPage>

Also fixes ~10 eslint errors

Copy link
Contributor

@liamcardenas liamcardenas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested and looks good.

CHANGELOG.md Outdated
@@ -16,6 +16,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/).
* App category for Linux ([#877](https://github.com/lbryio/lbry-app/pull/877))
* Add YouTube Sync reward ([#1147](https://github.com/lbryio/lbry-app/pull/1147))
* Retain previous screen sizing on startup ([#338](https://github.com/lbryio/lbry-app/issues/338))
* Add 'Go to page' input on channel pagination ([#1166](https://github.com/lbryio/lbry-app/pull/1166))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you request seems to be intertwined with another one somehow. Can you please figure out how to take out these pagination related changes?

@@ -4,6 +4,7 @@ import * as React from 'react';
import classnames from 'classnames';

type Props = {
centered?: boolean,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume this is from the pagination PR?

@@ -180,6 +183,14 @@ class FilePage extends React.Component<Props> {
<SubscribeButton uri={subscriptionUri} channelName={channelName} />
</React.Fragment>
)}
{speechSharable && (
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

have you run yarn format?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did, and it did not change anything

@@ -123,6 +123,9 @@ class FilePage extends React.Component<Props> {
if (channelName && channelClaimId) {
subscriptionUri = buildURI({ channelName, claimId: channelClaimId }, false);
}
const free = costInfo && costInfo.cost === 0;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does free need to be its own variable?

Copy link
Contributor

@liamcardenas liamcardenas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I meant to request changes

@daovist
Copy link
Contributor Author

daovist commented Mar 30, 2018

I am not sure what's going on with this conflict in src/renderer/scss/component/_form-field.scss

What Github tells me is a problem on lines 54-58 is not a problem locally

@lbry-bot lbry-bot assigned liamcardenas and unassigned liamcardenas Apr 3, 2018
@tzarebczan tzarebczan requested review from neb-b and removed request for liamcardenas May 10, 2018 01:26
@neb-b
Copy link

neb-b commented May 11, 2018

Can you rebase and get this up to date?

@tzarebczan
Copy link
Contributor

@seanyesmunt Tired of adding buttons to the show page yet? haha Wonder how this fits in with your changes to file actions. I'll try to give it a whirl today.

Copy link

@neb-b neb-b left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is awesome. I think the button placement is fine.

@neb-b neb-b merged commit 06956ec into lbryio:master May 30, 2018
infinite-persistence pushed a commit to infinite-persistence/lbry-desktop that referenced this pull request Apr 11, 2022
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

Successfully merging this pull request may close these issues.

None yet

4 participants