Refactor/home #71

ramirc5 · 2023-07-24T19:59:24Z

No description provided.

jex441 · 2023-07-24T20:00:42Z

src/Components/DOM/VoteStatusButton.js

@@ -61,6 +61,7 @@ export default function VoteStatusButton(props){
        }
        setVoteYesTotalState(voteYesTotal);
        setVoteNoTotalState(voteNoTotal);
+        console.log(tsrcPRStatusComponent, "tsrcPRStatusComponent");


jex441 · 2023-07-24T20:00:49Z

src/Components/DOM/VoteStatusButton.js

@@ -80,6 +81,7 @@ export default function VoteStatusButton(props){
        if(!tsrcPRStatus.mergeableCodeHost) {
          tsrcPRStatus.state = 'conflict';
        }
+        console.log(tsrcPRStatus, "tsrcPRStatus");


jex441 · 2023-07-24T20:01:04Z

src/Components/Extension/Home/Home.js

-    setSelectedPullRequestChildDefaultHash(pullRequest.childDefaultHash);
-    setSelectedPullRequestIssueID(pullRequest.issue_id);
+    setSelectedPullRequest({ ...pullRequest });
+    console.log(selectedPullRequest, "selected pull req")


jex441 · 2023-07-24T21:28:28Z

src/hooks/useRepoData.js

+            const fetchRepoData = async () => {
+            try {
+                const response = await postGetRepoData(`${owner}/${repo}`, ethereumAddress);
+                console.log("this is the response", response);  


jex441 · 2023-07-24T21:29:02Z

src/Components/Extension/SinglePullRequestView/VoteText.js

            setTextType('No');
        }

-        console.log("this is the text type", voted, chosenSide, disabled);
+        console.log("this is the text type", voted, side, disabled);


jex441 · 2023-07-24T21:29:22Z

src/Components/Extension/SinglePullRequestView/SinglePullRequestView.js

+        setDisabled(true);
+      }
+    }, [state]);
+    console.log(votePower, "vote power");


jex441 · 2023-07-25T15:36:37Z

src/Components/Extension/Home/TopInfoBar.js

+            </BoldText>
+            </OwnerRepo>
+            {onTurboSrc ? (
+            <VotePower>{votePowerAmount === 0 ? '0 votepower' : `${votePowerAmount} votepower`}</VotePower>


Capital V and P - VotePower

Also the font weight seems to change from Home to Single PR view in the Top Bar

jex441 · 2023-07-25T15:42:36Z

src/Components/Extension/Home/Home.js

-          </div>
+            </>
+          )}
+          {onTurboSrc ? null : loading ? (


This conditional is confusing

I think we can take out the onTurboSrc ? null : and just have the loading ? etc

jex441 · 2023-07-25T15:43:56Z

src/Components/Extension/Home/Home.js

 export default function Home() {
  const user = useSelector(state => state.auth.user);
  const repo = useSelector(state => state.repo.name);
  const owner = useSelector(state => state.repo.owner.login);
  const oldVersion = false;
-  const [pullRequests, setPullRequests] = useState([]);
-  const [tokenized, setTokenized] = useState(false);
  const [loading, setLoading] = useState(true);
  const [seeModal, setSeeModal] = useState(false);


Please change seeModal to [seeSinglePR, setSeeSinglePR]

jex441 · 2023-07-25T15:47:10Z

src/Components/Extension/Home/Home.js

 export default function Home() {
  const user = useSelector(state => state.auth.user);
  const repo = useSelector(state => state.repo.name);
  const owner = useSelector(state => state.repo.owner.login);
  const oldVersion = false;
-  const [pullRequests, setPullRequests] = useState([]);
-  const [tokenized, setTokenized] = useState(false);
  const [loading, setLoading] = useState(true);


Please declare the loading variable in the useGetRepoData hook and return it

It should be dependent on the response from useGetRepoData, so when the res is ready, set it to false

There could also be a variable called errorText in useGetRepoData which would set an error message to display in Home.js

.catch(error => if(error.status >= 400) {setErrorText('There was an error fetching the data')}

Update - this could be a stretch goal. We could do it later on

jex441 · 2023-07-25T15:50:12Z

src/Components/Extension/Home/Home.js

+            </>
+          )}
+          {onTurboSrc ? null : loading ? (
+            <SkeletonModal />


Remove Modal wording, SkeletonHome or something like that would be better

jex441 · 2023-07-25T15:55:28Z

src/Components/DOM/VoteStatusButton.js

@@ -63,7 +63,6 @@ export default function VoteStatusButton(props){
        setVoteNoTotalState(voteNoTotal);
        setTsrcPRStatus(tsrcPRStatusComponent);
      } catch (error) {
-        console.log('fetchVoteStatus error:', error)


I believe VoteStatusButton is outside of the scope of refactoring the extension Home page, also the console log is fine if it is console logging an error in a trycatch

jex441 · 2023-07-25T16:02:52Z

src/Components/Extension/SinglePullRequestView/SinglePullRequestView.js

-      setDisabled(true);
-    }
-  }, [state]);
+  const SinglePullRequestView = ({ selectedPullRequest, user, repo, githubToken, owner }) => {      


option + shift + f

jex441 · 2023-07-25T16:06:24Z

src/Components/Extension/SinglePullRequestView/VoteButtonGroup.js

opt + shift + f

jex441 · 2023-07-25T16:07:02Z

src/Components/Extension/SinglePullRequestView/VotesTable.js

opt + shift + f

jex441 · 2023-07-25T16:08:55Z

src/Components/Extension/SinglePullRequestView/VoteText.js

opt + shift + f

jex441 · 2023-07-25T16:09:59Z

src/hooks/useRepoData.js

+            }
+            };
+            fetchRepoData();
+        }, 500);


Can you experiment with taking out the setTimeout? It should work just with owner and repo as dependencies

jex441 · 2023-07-25T16:12:11Z

src/Components/Extension/Home/Home.js


  socket.on('vote received', function(ownerFromServer, repoFromServer, issueIDFromServer) {
    if (owner === ownerFromServer && repo === repoFromServer) {
-      getRepoDataHandler();
+      const { onTurboSrc, pullRequests, votePowerAmount } = useRepoData(owner, repo, ethereumAddress);


I think change the declarations above to be let instead of const

then just reassign them here

jex441 · 2023-07-25T16:12:18Z

src/Components/Extension/Home/Home.js

-    }, 500);
-    setPullRequestsLoaded(false);
-  }, [owner, repo]);
+  const { onTurboSrc, pullRequests, votePowerAmount } = useRepoData(owner, repo, user.ethereumAddress);


jex441 · 2023-07-25T16:13:50Z

src/Components/Extension/Home/Home.js

-
-  let [tokenAmount, setTokenAmount] = useState('');
-
-  let avatar = user?.avatar_url || null;

  useEffect(() => {
    //Set current logged in contributor/id to chrome storage for inject to verify user for voting
    chrome.storage.local.set({ contributor_name: user.login });
    chrome.storage.local.set({ contributor_id: user.ethereumAddress });
    setTimeout(() => setLoading(false), 1500);


I'd like to remove this setTimeout and have the loading be dependent on the res inside useGetRepoData

…duce code

jex441 · 2023-07-25T20:26:51Z

src/Components/Extension/Home/Home.js

-          <TopInfoBar owner={owner} repo={repo} votePowerAmount={votePowerAmount} onTurboSrc={onTurboSrc} />
-          {onTurboSrc && (
+          <TopInfoBar owner={owner} repo={repo} votePowerAmount={useCommas(votePowerAmount)} onTurboSrc={onTurboSrc} />
+          {onTurboSrc ? (


The logic here is showing the loader forever if the repo is not on turbosrc

The order for the conditional logic should be more intuitive.

If loading should be first as a switch case

if on turbosrc should be second

and if not on turbosrc should be last

Jeffrey Wood and others added 6 commits July 19, 2023 14:49

initial

29e7c90

testing refactor

e0f69c9

refactored setPullRequest()

0800402

split up some code in home.js to a new file

4b8b532

got custom hook working

e0f232a

merged refactor/useRepoData

3ec5cc4

ramirc5 requested a review from jex441 July 24, 2023 19:59

jex441 reviewed Jul 24, 2023

View reviewed changes

removed console.logs

815469c

jex441 reviewed Jul 24, 2023

View reviewed changes

removed console.logs

1003ebb

jex441 reviewed Jul 25, 2023

View reviewed changes

src/Components/Extension/SinglePullRequestView/VoteButtonGroup.js Outdated

Copy link

jex441 Jul 25, 2023

Choose a reason for hiding this comment

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

opt + shift + f

jex441 reviewed Jul 25, 2023

View reviewed changes

src/Components/Extension/SinglePullRequestView/VotesTable.js Outdated

Copy link

jex441 Jul 25, 2023

Choose a reason for hiding this comment

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

opt + shift + f

jex441 reviewed Jul 25, 2023

View reviewed changes

src/Components/Extension/SinglePullRequestView/VoteText.js

Copy link

jex441 Jul 25, 2023

Choose a reason for hiding this comment

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

opt + shift + f

jex441 reviewed Jul 25, 2023

View reviewed changes

moved loading to new hook and now using topinfobar where needed to re…

20f9bc7

…duce code

jex441 reviewed Jul 25, 2023

View reviewed changes

ramirc5 added 2 commits July 25, 2023 14:26

refactored home view logic

1941bf8

prettify

f4c6092

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor/home #71

Refactor/home #71

ramirc5 commented Jul 24, 2023

jex441 Jul 24, 2023

jex441 Jul 24, 2023

jex441 Jul 24, 2023

jex441 Jul 24, 2023

jex441 Jul 24, 2023

jex441 Jul 24, 2023

jex441 Jul 25, 2023

jex441 Jul 25, 2023

jex441 Jul 25, 2023

jex441 Jul 25, 2023

jex441 Jul 25, 2023

jex441 Jul 25, 2023

jex441 Jul 25, 2023

jex441 Jul 25, 2023

jex441 Jul 25, 2023

jex441 Jul 25, 2023

jex441 Jul 25, 2023

jex441 Jul 25, 2023

jex441 Jul 25, 2023

jex441 Jul 25, 2023

jex441 Jul 25, 2023

jex441 Jul 25, 2023

jex441 Jul 25, 2023

jex441 Jul 25, 2023

jex441 Jul 25, 2023 •

edited

Loading

Refactor/home #71

Are you sure you want to change the base?

Refactor/home #71

Conversation

ramirc5 commented Jul 24, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jex441 Jul 25, 2023 • edited Loading

Choose a reason for hiding this comment

jex441 Jul 25, 2023 •

edited

Loading