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

[Stats Refresh] Period Referrers: fix displaying details view #11741

Merged
merged 7 commits into from
May 21, 2019

Conversation

ScoutHarris
Copy link
Contributor

Ref #11360, #11080

This changes Period > Referrers to use UITableViewCells instead of one UIStackView.

Previously, all the data was stuffed into the stack view on the TopTotalsCell card. Now it only uses the TopTotalsCell card to show the header via DetailSubtitlesHeaderRow. The stack view is left empty. Instead, a UITableViewCell is added to the table for each data row via:

  • DetailExpandableRow - for parent rows
  • DetailExpandableChildRow - for child rows

And, because Referrers Search Engines rows can be expanded twice, "grandchild" rows are now supported.

This also addresses two other issues:
From #11080 - When no icon is available then show gridicons-globe.
From #11674 (comment) - fix gridicons-search icon size.

Update release notes:

  • If there are user facing changes, I have added an item to RELEASE-NOTES.txt.

To test:


  • Go to Stats > Period > Referrers.
  • For any Referrers that don't have an icon, verify the globe icon is displayed.

  • Select View more.
  • Verify the data appears relatively quickly.
  • For any Referrers that don't have an icon, verify the globe icon is displayed.
  • For a non-expandable row, verify row selection shows a web view.

  • For an expandable row, row selection shows all child rows.
  • When a row is expanded:
    • The child label text color is light grey.
    • Child rows have no separator line between them.
    • There is a full width separator line above the parent row, and below the last child.
  • Selecting a child row shows a web view.

expanded


  • For the Search Engines row: (tip: en.blog has this row)
    • Row selection shows all child rows.
    • If a child row is expandable, it looks like a parent row, i.e. text color is dark grey, and the chevron is pointed down.
    • When a child row is expanded, it exhibits the same behavior has listed above (For an expandable row...).

search_engines_init

search_engines_expanded

@ScoutHarris ScoutHarris added this to the 12.6 milestone May 21, 2019
@ScoutHarris ScoutHarris requested a review from a user May 21, 2019 20:25
@ScoutHarris ScoutHarris self-assigned this May 21, 2019
@ScoutHarris ScoutHarris mentioned this pull request May 21, 2019
45 tasks
downloadImageFrom(iconURL)
}

if let iconURL = rowData.userIconURL {
imageWidthConstraint.constant = Constants.userImageSize
imageView.layer.cornerRadius = Constants.userImageSize * 0.5
imageView.layer.cornerRadius = imageSize * 0.5
Copy link

Choose a reason for hiding this comment

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

I know this code was in place previously, but it occurred to me that some might not understand the intent of this message.

If we're making the image view circular, it might be more clear to set the path for a mask via one of the following methods:

  • init(arcCenter:radius:startAngle:endAngle:clockwise:)
  • init(ovalIn:)

I don't feel strongly about this, but wanted to share.

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

@ScoutHarris this is working as described for me! :shipit:

@ScoutHarris ScoutHarris merged commit 33e58ff into develop May 21, 2019
@ScoutHarris ScoutHarris deleted the fix/11360-referrers_details branch May 21, 2019 22:56
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.

1 participant