Skip to content

Issue/update pages item ui - #11257

Merged
jd-alexander merged 4 commits into
feature/master-pages-offline-supportfrom
issue/update-pages-item-ui
Feb 12, 2020
Merged

Issue/update pages item ui#11257
jd-alexander merged 4 commits into
feature/master-pages-offline-supportfrom
issue/update-pages-item-ui

Conversation

@malinajirka

@malinajirka malinajirka commented Feb 7, 2020

Copy link
Copy Markdown
Contributor

Partially fixes #11133

Merge instructions:

  1. Offline Pages : PageList Item Progress State  #11191 needs to be merged first.
  2. Update target branch to master_page_offline_support
  3. Merge

Updates the layout of the PageList list items to match the layout of PostList items. I've copy pasted the post list compact item layout so there shouldn't be any issues.

Before After
Screenshot_1581080900 Screenshot_1581080819
Screenshot_1581080892 Screenshot_1581080812

To test:
We'll continue working on the PageList. So I don't think we need to do detailed tests.

  • Open Page List and make sure the items look ok
  • Try to upload a post and make sure the progress bar looks ok
  • Make a change to the post and leave the editor without saving the chagnes ->make sure the "Local changes" label looks ok

PR submission checklist:

  • I have considered adding unit tests where possible.
  • I have considered adding accessibility improvements for my changes.
  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

cc @mbshakti Could you please just quickly check the design. I think it should exactly match the design on PostList, but just to be sure I didn't overlook something (one difference is, that the "Local changes" label isn't yellow - we plan to add this in a different PR). Thanks!

@peril-wordpress-mobile

peril-wordpress-mobile Bot commented Feb 7, 2020

Copy link
Copy Markdown

You can test the changes on this Pull Request by downloading the APK here.

@malinajirka malinajirka added [Status] Needs Design Review A designer needs to sign off on the implemented design. [Status] Needs Code Review labels Feb 7, 2020
@jd-alexander

Copy link
Copy Markdown
Contributor

I reviewed this and it works as expected! I’m going to review again today. The design review has to be completed before this is merged right ?

@malinajirka

Copy link
Copy Markdown
Contributor Author

The design review has to be completed before this is merged right ?

I think that'd be ideal since I didn't confirm this change with a designer. It just felt weird the two list were looking differently + I needed more space for the labels.

@malinajirka

Copy link
Copy Markdown
Contributor Author

@jd-alexander I've talked to Shakti and I'd suggest merging this PR since other PRs are based on this branch. We are merging it into a working branch so it won't get released to our users before the design review is done. Thanks!

@jd-alexander jd-alexander left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Awesome! LGTM 🚢

@jd-alexander
jd-alexander changed the base branch from issue/page_progress_bar to feature/master-pages-offline-support February 12, 2020 20:07
@jd-alexander
jd-alexander merged commit 4f9cb12 into feature/master-pages-offline-support Feb 12, 2020
@jd-alexander
jd-alexander deleted the issue/update-pages-item-ui branch February 12, 2020 20:08
@mbshakti

Copy link
Copy Markdown

The rows match post lists - lgtm!

@malinajirka malinajirka modified the milestones: 14.3, 14.6 Mar 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Posts + Pages [Status] Needs Design Review A designer needs to sign off on the implemented design. [Type] Enhancement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants