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.

Sign up for GitHub

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

Jump to bottom

Lint: fix client/reader #25921

Merged
merged 5 commits into from
Jul 30, 2018
Merged

Lint: fix client/reader #25921

merged 5 commits into from
Jul 30, 2018

Conversation

bluefuton
Copy link
Contributor

@bluefuton bluefuton commented Jul 9, 2018 •
edited
Loading

Part of the grand effort to fix all the eslint errors (#24504).

This PR covers client/reader/*. I'm using this to find them:

npx eslint client/reader/* --ext js,jsx

Remaining larger issues that we'll address in followup PRs:

  • Refactor ReaderSidebar to ES6 component
  • Remove componentWillReceiveProps usage
  • Remove componentWillMount usage
  • Use button element in intro banners and update pill

@bluefuton bluefuton added this to the Reader: Bug Backlog milestone Jul 9, 2018
@matticbot
Copy link
Contributor

Test live: https://calypso.live/?branch=fix/reader/lint-client-reader

@bluefuton bluefuton self-assigned this Jul 9, 2018
@blowery blowery mentioned this pull request Jul 9, 2018
Closed
@bluefuton bluefuton added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. and removed [Status] In Progress labels Jul 9, 2018
@bluefuton bluefuton requested a review from blowery July 9, 2018 05:52
@bluefuton bluefuton added [Status] In Progress and removed [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Jul 9, 2018
@bluefuton
Copy link
Contributor Author

bluefuton commented Jul 9, 2018 •
edited
Loading

Visual fix on the intro banners needed after converting div to button:

screen shot 2018-07-09 at 17 56 17

The update pill arrow needs some minor tuning as well.

@bluefuton
Copy link
Contributor Author

I'll address the button element + position fixes in a followup PR instead.

@bluefuton bluefuton added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. and removed [Status] In Progress labels Jul 9, 2018
@bluefuton bluefuton requested review from jsnmoon and a team July 11, 2018 04:05
@alisterscott
Copy link
Contributor

I had a click through Reader on this branch and couldn't see any obvious regressions or visual issues

alisterscott
alisterscott approved these changes Jul 30, 2018
View reviewed changes
Copy link
Contributor

@alisterscott alisterscott left a comment

Choose a reason for hiding this comment

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

LGTM

@bluefuton bluefuton force-pushed the fix/reader/lint-client-reader branch from f8c3706 to 7421332 Compare July 30, 2018 05:12
@bluefuton bluefuton merged commit 9b31efb into master Jul 30, 2018
@bluefuton bluefuton deleted the fix/reader/lint-client-reader branch July 30, 2018 05:20
@matticbot matticbot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Jul 30, 2018
@bluefuton bluefuton mentioned this pull request Jul 31, 2018
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alisterscott alisterscott alisterscott approved these changes

@blowery blowery Awaiting requested review from blowery

@jsnmoon jsnmoon Awaiting requested review from jsnmoon

Assignees

@bluefuton bluefuton

Labels
[Feature] Reader The reader site on Calypso. [Type] Janitorial
Projects
None yet
Milestone
Reader: Bug Backlog
Development

Successfully merging this pull request may close these issues.
3 participants
@bluefuton @matticbot @alisterscott