Refactor toolbar into v-authorship tab #566

ongspxm · 2019-02-21T06:53:04Z

might affect #524 and #544

Currently the toolbar for the "expand/collapse all" text is outside of
the v-authorship tab. This shouldn't be the case as it makes it
difficult to extend and include other kinds of tabs.

Let's shift the toolbar into the v-authorship tab, and make some
corresponding changes to how the toggle state is changed.

This will keep the toolbar within the authorship tab, which makes sense
as the button is only relevant for the authorship tab. In light that we
are going to have other kinds of tabs to extend the use of the side tab
in the future.

ongspxm · 2019-02-22T15:26:02Z

@apoorva17 could you check if there is any test case i missed?

chel-seyy

The Expand/Collapse All tab does not seem to reflect the changes (files are already expanded)

frontend/src/index.jade

ongspxm · 2019-02-26T01:54:49Z

The Expand/Collapse All tab does not seem to reflect the changes

@chelseyong which one is this? i tested everything seems fine. can you confirm this for the lastest netlify?

eugenepeh · 2019-03-03T11:45:02Z

frontend/src/static/js/main.js

@@ -99,10 +99,11 @@ window.app = new window.Vue({

    isLoading: false,
    isCollapsed: false,
+
+    // isTabActive used to force tab wrapper to load
    isTabActive: true,


perhaps inline the comments? E.g. isTabActive: true, // to force tab wrapper to load

eugenepeh · 2019-03-03T11:46:09Z

frontend/src/static/js/v_authorship.js

+    },
+
+    updateCount() {
+      this.activeFileCount = document.getElementsByClassName('file active').length;


activeFilesCount

eugenepeh · 2019-03-03T11:46:46Z

frontend/src/index.jade

@@ -37,17 +37,14 @@ html

        #tabs-wrapper(v-if="isTabActive")
          #tab-resize(onmousedown="registerMouseMove()")
-          .tab-close(v-on:click="isTabActive=false")
+          .tab-close(v-on:click="isTabActive=false;")


this change doesn't seems to match the purpose of this PR

eugenepeh · 2019-03-06T23:03:56Z

frontend/src/index.jade

-                  v-bind:key="generateKey(tabInfo.tabAuthorship)",
-                  v-bind:info="tabInfo.tabAuthorship")
+              v_authorship#tab-authorship.tab-pane(
+                v-if="tabType=='authorship'"


space around operator, also, why not follow existing style to use ===?

eugenepeh · 2019-03-06T23:04:05Z

frontend/src/index.jade

@@ -166,6 +163,10 @@ html

    vuetemplate#v_authorship
      #authorship
+        // TODO: remove in #524
+        .toolbar(v-if="info.totalCommits > 0")
+            a(v-if="activeFileCount===0", v-on:click="expandAll(true)") Expand all


space around operator ===

frontend/src/static/js/main.js

ongspxm · 2019-03-12T02:29:29Z

@yamidark Merge?

yamidark · 2019-03-12T13:30:18Z

@ongspxm We just merged in #487, seems like there is a small conflict, can help to resolve it?

yamidark · 2019-03-12T16:59:22Z

frontend/src/index.jade

@@ -199,6 +196,10 @@ html

    vuetemplate#v_authorship
      #authorship
+        // TODO: remove in #524
+        .toolbar(v-if="info.totalCommits > 0")
+            a(v-if="activeFileCount === 0", v-on:click="expandAll(true)") Expand all


Seems like this activeFileCount variable hasn't been refactored to latest master?

ongspxm · 2019-03-12T23:56:32Z

sha? let me merge again

…

On Wed, 13 Mar 2019 at 00:59, Jun An ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In frontend/src/index.jade <#566 (comment)>: > @@ -199,6 +196,10 @@ html vuetemplate#v_authorship #authorship + // TODO: remove in #524 + .toolbar(v-if="info.totalCommits > 0") + a(v-if="activeFileCount === 0", v-on:click="expandAll(true)") Expand all [image: Untitled] <https://user-images.githubusercontent.com/18352498/54219855-2e0c9e80-452b-11e9-99d3-469577c6fa26.png> Seems like this activeFileCount variable hasn't been refactored to latest master? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#566 (review)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABXVRuZ1Pr3dGdiF6Hj6O8eFDBJat0y2ks5vV9zrgaJpZM4bGxUz> .

ongspxm added 2 commits February 21, 2019 14:51

wip

a387e19

deactivating the tab

4fc1151

ongspxm changed the title ~~refactoring hows tabs works~~ refactoring expandAll/ collapseAll toolbar for authorship Feb 21, 2019

ongspxm added 5 commits February 21, 2019 15:43

remove js for button update

3fcd62b

move expand func to v_authorship

9e73331

wip. left with updateCount

c66a3ee

updateCount using document.getElementsByClassName

7dbb41b

wrap everything within v-authorship

b813aa2

ongspxm changed the title ~~refactoring expandAll/ collapseAll toolbar for authorship~~ refactoring to keep toolbar within v-authorship Feb 21, 2019

fix lint

a7aeefd

ongspxm requested review from apoorva17, chel-seyy, yamidark and yong24s and removed request for apoorva17 February 21, 2019 14:39

ongspxm added the s.ToReview label Feb 21, 2019

updated to use isTabActive instead of tabActive to trigger tab display

4bb21d6

yamidark approved these changes Feb 22, 2019

View reviewed changes

chel-seyy suggested changes Feb 23, 2019

View reviewed changes

frontend/src/index.jade Show resolved Hide resolved

This was referenced Feb 24, 2019

[#465] Bookmark codeview uri #524

Merged

Report: Add zoom in feature for ramp charts #573

Merged

added TODO to remove change

b632566

ongspxm mentioned this pull request Feb 24, 2019

refactoring the way we handle sidebar opening events #574

Merged

Merge branch 'master' into tabs

6b718c7

yamidark added s.Ongoing and removed s.ToReview labels Feb 25, 2019

ongspxm removed the request for review from apoorva17 February 26, 2019 02:06

eugenepeh reviewed Mar 3, 2019

View reviewed changes

minor nits

69370e6

yamidark requested a review from eugenepeh March 5, 2019 14:17

eugenepeh reviewed Mar 6, 2019

View reviewed changes

ongspxm added 2 commits March 7, 2019 15:11

Merge branch 'master' into tabs

fce473d

minor nits on equality signs

2bd1a05

eugenepeh approved these changes Mar 10, 2019

View reviewed changes

frontend/src/static/js/main.js Show resolved Hide resolved

minor nit

e20002b

ongspxm added s.FinalReview and removed s.ToReview labels Mar 12, 2019

ongspxm requested review from eugenepeh, yamidark and a team and removed request for apoorva17, eugenepeh and yamidark March 12, 2019 02:28

Merge branch 'master' into tabs

74144b0

yamidark changed the title ~~refactoring to keep toolbar within v-authorship~~ Refactor toolbar into v-authorship tab Mar 12, 2019

yamidark added s.ToMerge and removed s.FinalReview labels Mar 12, 2019

yamidark removed the request for review from a team March 12, 2019 12:03

Merge branch 'master' into tabs

50edcaa

yamidark reviewed Mar 12, 2019

View reviewed changes

ongspxm added 2 commits March 13, 2019 07:59

fix: err in merge

0998b04

Merge branch 'tabs' of github.com:ongspxm/reposense into tabs

4e3ed4a

yamidark approved these changes Mar 13, 2019

View reviewed changes

yamidark merged commit daaed30 into reposense:master Mar 13, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor toolbar into v-authorship tab #566

Refactor toolbar into v-authorship tab #566

ongspxm commented Feb 21, 2019 •

edited

ongspxm commented Feb 22, 2019

chel-seyy left a comment

ongspxm commented Feb 26, 2019 •

edited

eugenepeh Mar 3, 2019

eugenepeh Mar 3, 2019

eugenepeh Mar 3, 2019 •

edited

eugenepeh Mar 6, 2019

eugenepeh Mar 6, 2019

ongspxm commented Mar 12, 2019

yamidark commented Mar 12, 2019

yamidark Mar 12, 2019

ongspxm commented Mar 12, 2019 via email

Refactor toolbar into v-authorship tab #566

Refactor toolbar into v-authorship tab #566

Conversation

ongspxm commented Feb 21, 2019 • edited

ongspxm commented Feb 22, 2019

chel-seyy left a comment

Choose a reason for hiding this comment

ongspxm commented Feb 26, 2019 • edited

eugenepeh Mar 3, 2019

Choose a reason for hiding this comment

eugenepeh Mar 3, 2019

Choose a reason for hiding this comment

eugenepeh Mar 3, 2019 • edited

Choose a reason for hiding this comment

eugenepeh Mar 6, 2019

Choose a reason for hiding this comment

eugenepeh Mar 6, 2019

Choose a reason for hiding this comment

ongspxm commented Mar 12, 2019

yamidark commented Mar 12, 2019

yamidark Mar 12, 2019

Choose a reason for hiding this comment

ongspxm commented Mar 12, 2019 via email

ongspxm commented Feb 21, 2019 •

edited

ongspxm commented Feb 26, 2019 •

edited

eugenepeh Mar 3, 2019 •

edited