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

Updated Zulip.js #180

Closed
wants to merge 6 commits into from
Closed

Updated Zulip.js #180

wants to merge 6 commits into from

Conversation

msaini28r
Copy link

@msaini28r msaini28r commented Oct 15, 2021

Description

Fixes #72

Type of Change:

  • Code
  • Quality Assurance
  • User Interface
  • Outreach
  • Documentation

Code/Quality Assurance Only

  • Bug fix (non-breaking change which fixes an issue)
  • This change requires a documentation update (software upgrade on readme file)
  • New feature (non-breaking change which adds functionality pre-approved by mentors)

How Has This Been Tested?

Checklist:

  • My PR follows the style guidelines of this project
  • I have performed a self-review of my own code or materials
  • I have commented my code or provided relevant documentation, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • Any dependent changes have been merged

I think the issue is solved. #72
@msaini28r
Copy link
Author

The issue is solved. #72

Copy link
Member

@codesankalp codesankalp left a comment

Choose a reason for hiding this comment

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

See my comments @msaini0r 👇

@@ -59,10 +59,6 @@ class Zulip extends Component {
<b>Streams: </b>
<br />
<ul>
<li>
<b>#newcomers: </b>
{zulipstat[0] ? zulipstat[0].newcomers_messages : 0}
Copy link
Member

Choose a reason for hiding this comment

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

Why are you removing this?
You need to change the condition so that it can render without any errors.

{zulipstat && zulipstat[0] && zulipstat[0].newcomers_message ? zulipstat[0].newcomers_message : 0}

Same change for other lines also.

Copy link
Author

Choose a reason for hiding this comment

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

@codesankalp can you please check if I made the right changes?

@@ -1,121 +0,0 @@
import React, { Component } from 'react';
import { connect } from 'react-redux';
Copy link
Member

Choose a reason for hiding this comment

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

@msaini0r You have removed the whole code from this file. Please revert it.

Copy link
Author

Choose a reason for hiding this comment

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

@can you give me instructions how to revert it. @codesankalp

Copy link
Author

Choose a reason for hiding this comment

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

@codesankalp should i delete that branch?

@codesankalp codesankalp added the Status: Changes Requested Changes are required to be done by the PR author. label Oct 15, 2021
This reverts commit 9593b3f.
@@ -61,23 +61,23 @@ class Zulip extends Component {
<ul>
<li>
<b>#newcomers: </b>
{zulipstat[0] ? zulipstat[0].newcomers_messages : 0}
{zulipstat && zulipstat[0] && zulipstat[0].newcomers_message ? zulipstat[0].newcomers_message : 0}
Copy link
Member

Choose a reason for hiding this comment

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

This line looks repeated for zulipstat && zulipstat[0]. Can you please remove this zulipstat && zulipstat[0] from each line and add zulipstat && zulipstat[0] in parent tag (ul)?

Suggested change
{zulipstat && zulipstat[0] && zulipstat[0].newcomers_message ? zulipstat[0].newcomers_message : 0}
{zulipstat[0].newcomers_message ? zulipstat[0].newcomers_message : 0}

Copy link
Author

Choose a reason for hiding this comment

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

@codesankalp check now changes done.

@@ -58,26 +58,26 @@ class Zulip extends Component {
<>
<b>Streams: </b>
<br />
<ul>
<zulipstat && zulipstat[0]>
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
<zulipstat && zulipstat[0]>
{zulipstat && zulipstat[0] ? <ul>below code<ul> : null}

Copy link
Author

@msaini28r msaini28r left a comment

Choose a reason for hiding this comment

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

@codesankalp see!!

@codesankalp codesankalp added Status: Needs Review PR needs an additional review or a maintainer's review. and removed Status: Changes Requested Changes are required to be done by the PR author. labels Oct 16, 2021
@Aaishpra Aaishpra added Category: Coding Changes to code base or refactored code that doesn't fix a bug. hacktoberfest-accepted Status: Ready to Merge Work has been tested and needs a final review and merge from a repo maintainer. and removed Status: Needs Review PR needs an additional review or a maintainer's review. labels Oct 16, 2021
@isabelcosta
Copy link
Member

@msaini0r please edit the pull request description with a description of the changes and how you tested this? This is a good practice for future PRs, whether it's in our community or other projects, and good for documentation purposes 🤗

Copy link
Member

@isabelcosta isabelcosta left a comment

Choose a reason for hiding this comment

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

@msaini0r could you please check why the PR check is failing? This seems to be related to your PR. Once you fix the failing check I can approve this PR and merge it.

@isabelcosta isabelcosta removed the Status: Ready to Merge Work has been tested and needs a final review and merge from a repo maintainer. label Oct 17, 2021
@isabelcosta
Copy link
Member

removed Ready to Merge label as the PR checks are failing cc @Aaishpra

@msaini28r
Copy link
Author

@isabelcosta I am not able to find the issue. I would like to worrk on some other issues

@Aaishpra
Copy link
Member

@isabelcosta I am not able to find the issue. I would like to worrk on some other issues

Hi @msaini0r only linters and formatters check is failing if you can fix them it would be great
Try running npm run lint to fix them
Pasting the error log here so you can have a look.

 
Linters and Formatters
failed 18 hours ago in 58s
Search logs
3s
1s
3s
47s
4s
Run npm run lint

> [email protected] lint /home/runner/work/anitab-forms-web/anitab-forms-web
> eslint .


/home/runner/work/anitab-forms-web/anitab-forms-web/src/components/Zulip.js
Error:   61:50  error  Insert `(⏎······················`                                                                                                                                                                                                                                             prettier/prettier
Error:   62:23  error  Insert `··`                                                                                                                                                                                                                                                                   prettier/prettier
Error:   63:1   error  Insert `··`                                                                                                                                                                                                                                                                   prettier/prettier
Error:   64:25  error  Replace `{zulipstat[0].newcomers_message·?·zulipstat[0].newcomers_message` with `··{zulipstat[0].newcomers_message⏎····························?·zulipstat[0].newcomers_message⏎···························`                                                                  prettier/prettier
Error:   65:1   error  Insert `··`                                                                                                                                                                                                                                                                   prettier/prettier
Error:   66:23  error  Insert `··`                                                                                                                                                                                                                                                                   prettier/prettier
Error:   67:1   error  Insert `··`                                                                                                                                                                                                                                                                   prettier/prettier
Error:   68:1   error  Replace `························{zulipstat[0].celebrate_message·?·zulipstat[0].celebrate_message` with `··························{zulipstat[0].celebrate_message⏎····························?·zulipstat[0].celebrate_message⏎···························`                  prettier/prettier
Error:   69:1   error  Replace `······················` with `························`                                                                                                                                                                                                              prettier/prettier
Error:   70:1   error  Insert `··`                                                                                                                                                                                                                                                                   prettier/prettier
Error:   71:25  error  Insert `··`                                                                                                                                                                                                                                                                   prettier/prettier
Error:   72:1   error  Replace `························{zulipstat[0].opportunities_message·?·zulipstat[0].opportunities_message` with `··························{zulipstat[0].opportunities_message⏎····························?·zulipstat[0].opportunities_message⏎···························`  prettier/prettier
Error:   73:1   error  Insert `··`                                                                                                                                                                                                                                                                   prettier/prettier
Error:   74:23  error  Insert `··`                                                                                                                                                                                                                                                                   prettier/prettier
Error:   75:1   error  Insert `··`                                                                                                                                                                                                                                                                   prettier/prettier
Error:   76:25  error  Replace `{zulipstat[0].questions_message·?·zulipstat[0].questions_message` with `··{zulipstat[0].questions_message⏎····························?·zulipstat[0].questions_message⏎···························`                                                                  prettier/prettier
Error:   77:1   error  Insert `··`                                                                                                                                                                                                                                                                   prettier/prettier
Error:   78:23  error  Insert `··`                                                                                                                                                                                                                                                                   prettier/prettier
Error:   79:1   error  Replace `························` with `··························`                                                                                                                                                                                                          prettier/prettier
Error:   80:1   error  Replace `·······················{zulipstat[0].general_message·?·zulipstat[0].general_message` with `··························{zulipstat[0].general_message⏎····························?·zulipstat[0].general_message⏎···························`                           prettier/prettier
Error:   81:1   error  Insert `··`                                                                                                                                                                                                                                                                   prettier/prettier
Error:   82:1   error  Replace `····················</ul>` with `······················</ul>⏎····················)`                                                                                                                                                                                  prettier/prettier`

@msaini28r
Copy link
Author

@Aaishpra already tried npm run lint but it is showing me an error.

@msaini28r msaini28r closed this Oct 19, 2021
@msaini28r msaini28r deleted the patch-1 branch October 19, 2021 16:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category: Coding Changes to code base or refactored code that doesn't fix a bug. hacktoberfest-accepted
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug: Streamwise messages not showing on zulip stat card to admin
4 participants