Skip to content
This repository has been archived by the owner on May 13, 2024. It is now read-only.

Publisher status refresh now shows confirmation #475

Merged
merged 1 commit into from
May 29, 2019

Conversation

jasonrsadler
Copy link
Contributor

@jasonrsadler jasonrsadler commented May 24, 2019

Changes

Partial for brave/brave-browser#4351
Publisher panel refresh now has a confirmation signal to let the user know that the check is complete.

Test plan

in brave-core

Link / storybook path to visual changes

Integration

  • Does this contain changes to src/components or src/

    • Will you publish to npm immediately after this PR, or wait until sometime in the future?
    • Incompatible API change to something existing (major version increase)
    • Adding new backwards-compatible functionality? (minor version increase)
    • Fixing a bug backwards-compatibly? (patch version increase)
  • Does this contain changes to src/features for brave-core?

    • Are there non backwards-compatible changes required for brave-core? Do not merge until brave-core PR is approvable. Link to brave-core PR:
    • Will you create brave-core PR to update to this commit after it is merged?
    • Wants uplift to brave-core feature branch?
      • When uplift-approved, merge to brave-core-0.VV.x feature branch
      • Create additional brave-core PRs for each feature branch to update commit

@@ -312,6 +312,11 @@ storiesOf('Feature Components/Rewards/Concepts/Desktop', module)

const onRefreshPublisher = () => {
store.set({ refreshingPublisher: !store.state.refreshingPublisher })
setTimeout(() => {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: can we just pass a single object with three props to store.set?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

to {
opacity: 0;
}
`
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice transition work 👍

@@ -97,6 +105,7 @@ export default class Profile extends React.PureComponent<Props, {}> {
) : null}
</StyledTitleWrap>
{verified && type === 'big' ? (
!refreshingPublisher && !publisherRefreshed ?
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we split all code bellow (that is inside StyledContent) into separate function? This way function will tell us more what is going on, because current code is quire hard to read with nested if statements

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@jasonrsadler jasonrsadler force-pushed the pub-refresh-illus branch 2 times, most recently from cc1a24e to cf7ee70 Compare May 28, 2019 04:05
@jasonrsadler jasonrsadler requested review from NejcZdovc and ryanml May 28, 2019 04:06
@jasonrsadler jasonrsadler dismissed NejcZdovc’s stale review May 28, 2019 04:06

Comments addressed

@jasonrsadler jasonrsadler force-pushed the pub-refresh-illus branch 2 times, most recently from 2c93266 to 470992d Compare May 28, 2019 15:45
@jasonrsadler jasonrsadler merged commit cedd99a into master May 29, 2019
@NejcZdovc NejcZdovc deleted the pub-refresh-illus branch May 29, 2019 14:41
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants