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

activity stream DRY & cleanup #5041

Merged
merged 6 commits into from
Aug 22, 2019
Merged

Conversation

owocki
Copy link
Contributor

@owocki owocki commented Aug 19, 2019

Description

massive activity stream cleanup

  1. DRY's the /activity page and the profile page. (5 files down to 1, and a bunch of deleted code! yay)
  2. responsive cleanups
  3. a few small cleanups elsewhere from testing my local prod backup.
Refers/Fixes

self

Testing

tested locally.. here is a screenshot

Screen Shot 2019-08-20 at 12 50 48 AM

Screen Shot 2019-08-20 at 12 50 14 AM

@owocki owocki changed the title massive activity stream cleanup activity stream DRY & cleanup Aug 19, 2019
@codecov
Copy link

codecov bot commented Aug 19, 2019

Codecov Report

Merging #5041 into master will decrease coverage by 0.02%.
The diff coverage is 6.25%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #5041      +/-   ##
=========================================
- Coverage   30.92%   30.9%   -0.03%     
=========================================
  Files         217     217              
  Lines       17459   17474      +15     
  Branches     2393    2400       +7     
=========================================
+ Hits         5400    5401       +1     
- Misses      11839   11852      +13     
- Partials      220     221       +1
Impacted Files Coverage Δ
app/dashboard/views.py 14.01% <0%> (ø) ⬆️
app/dashboard/models.py 56.6% <6.66%> (-0.43%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3612036...5692c37. Read the comment docs.

@codecov
Copy link

codecov bot commented Aug 19, 2019

Codecov Report

Merging #5041 into master will decrease coverage by 0.02%.
The diff coverage is 6.25%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5041      +/-   ##
==========================================
- Coverage   30.89%   30.87%   -0.03%     
==========================================
  Files         218      218              
  Lines       17476    17491      +15     
  Branches     2394     2401       +7     
==========================================
+ Hits         5400     5401       +1     
- Misses      11856    11870      +14     
  Partials      220      220
Impacted Files Coverage Δ
app/dashboard/views.py 14.01% <0%> (ø) ⬆️
app/dashboard/models.py 56.6% <6.66%> (-0.43%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1b63315...a938c7b. Read the comment docs.

{% elif row.metadata.grant_logo %}
<img class="avatar" src="{{ row.metadata.grant_logo }}" style="width: 30px; height: 30px; bottom: 0; right: 0; position: absolute;"/>
{% if row.secondary_avatar_url %}
<img class="avatar secondary_avatar" src="{{row.secondary_avatar_url}}" />
Copy link
Member

Choose a reason for hiding this comment

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

this is nice ❤️

Copy link
Contributor Author

Choose a reason for hiding this comment

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

DRY ftw

thelostone-mc
thelostone-mc previously approved these changes Aug 20, 2019
@@ -1,5 +1,5 @@

#activity_stream .avatar {
.activity_stream .avatar {
Copy link
Contributor

Choose a reason for hiding this comment

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

yay

octavioamu
octavioamu previously approved these changes Aug 21, 2019
Copy link
Contributor

@octavioamu octavioamu left a comment

Choose a reason for hiding this comment

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

Looks great, just travis complaining and will be good to go!

right: 0rem;
position: absolute;
}

.funder-avatar,
.activity-avatar {
text-align: center;
}

.funder-avatar,
Copy link
Contributor

Choose a reason for hiding this comment

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

this can be removed I think

Copy link
Contributor

Choose a reason for hiding this comment

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

looks like this is the reason why travis is complaining
34:15 ✖ Unexpected empty block block-no-empty

Copy link
Contributor Author

Choose a reason for hiding this comment

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

just pushed up a fix

@owocki owocki dismissed stale reviews from octavioamu and thelostone-mc via a938c7b August 21, 2019 15:43
Copy link
Contributor

@octavioamu octavioamu left a comment

Choose a reason for hiding this comment

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

lgtm

@thelostone-mc thelostone-mc merged commit 1cb0949 into master Aug 22, 2019
@thelostone-mc thelostone-mc deleted the kevin/activity_stream_cleanup branch June 27, 2020 00:48
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.

3 participants