Git Merge Request #267: [#8216] Personal Dashboard - Create Activity Section (merged)

Merging...

Merged

Something went wrong. Please, merge manually

Checking if merge is possible...

Something went wrong. Please, merge manually

Deshani wants to merge 2 commits from /u/deshani/allura-personal-dashboard/ to master, 2018-07-09

Commit Date  
[b05bec] (followers_section) by deshanigtk deshanigtk

[#8216] Personal Dashboard - Activity Section cleaning up the code

 2018-07-09 17:20:06 Tree
[e5c514] by deshanigtk deshanigtk

[#8216] Personal Dashboard - Create Followers Section

 2018-07-06 18:53:53 Tree

Discussion

  • Dave Brondsema

    Dave Brondsema - 2018-07-09

    This is look good, really no problems I've found, but some cleanup can be done to keep the code smaller and simpler - always good :)

    • The {% block actions %} section will never run since the current user is always the user. (It runs on Activity tools when you look at another person's/project's activity). So that can be removed.
    • And then the follow_toggle and following variables aren't used, so the code for them can be removed. (Which is good anyway since those had trailing commas like FollowToggle(), which would make them incorrectly be a tuple)
    • the follow.js javascript code doesn't do anything (it runs on $('.artifact_follow') elements and there aren't any on the page). So we can omit the config dir and register_ew_resources and g.register_js, etc.
    • the activity.html template is virtually the same as the ForgeActivity profile_section.html template. And ActivitySection.prepare_context is very similar to ForgeActivityProfileSection.prepare_context, although it will have some differences now. I think it'd be worth updating the templates so the ~20 lines of HTML isn't duplicated. I think the best way would be to move everything that is within {% block content %} into the forgeactivity templates/macros.html file as a new macro. That macro file is already imported into both of the templates. Then within the block content section of both files you just have to call the macro something like am.timeline_section(timeline, activity_app) or whatever you name it.
     

    • Deshani

      Deshani - 2018-07-09

      Thanks for the feedback. I've done the changes and updated

       

  • Dave Brondsema

    Dave Brondsema - 2018-07-09
    • Status: open --> merged
     

Log in to post a comment.