Skip to content
This repository has been archived by the owner on Aug 7, 2024. It is now read-only.

feat: remove public stats #4641 #4642

Merged
merged 3 commits into from
Feb 10, 2023
Merged

feat: remove public stats #4641 #4642

merged 3 commits into from
Feb 10, 2023

Conversation

eddiejaoude
Copy link
Member

@eddiejaoude eddiejaoude commented Feb 9, 2023 •
edited
Loading

Fixes Issue

closes #4641

Changes proposed

Check List (Check all the applicable boxes)

  • My code follows the code style of this project.
  • My change requires changes to the documentation.
  • I have updated the documentation accordingly.
  • All new and existing tests passed.
  • This PR does not contain plagiarized content.
  • The title of my pull request is a short description of the requested changes.

Screenshots

Note to reviewers

github-actions[bot]
github-actions bot reviewed Feb 9, 2023
View reviewed changes
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

It's great having you contribute to this project

Welcome to the community 🤓

If you would like to continue contributing to open source and would like to do it with an awesome inclusive community, you should join our Discord chat and our GitHub Organisation - we help and encourage each other to contribute to open source little and often 🤓 . Any questions let us know.

@github-actions github-actions bot added large Pull request with more than 30 changed lines tests storybook labels Feb 9, 2023
gptkrsh
gptkrsh reviewed Feb 9, 2023
View reviewed changes
Copy link
Contributor

@gptkrsh gptkrsh left a comment

Choose a reason for hiding this comment

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

Also: Do we use the abbreviate Number function anywhere else since it's not removed?

components/navbar/Navbar.js
@@ -171,7 +167,14 @@ export default function Navbar() {
</div>
</div>

<div className={`${isOpen?"transform translate-y-0 opacity-100":"transform -translate-y-96 opacity-0 "} md:hidden z-20 absolute t-0 bg-gray-800 transition-all duration-700 ease-in-out w-full`} id="mobile-menu">
<div
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this just formatting or something changes here 🤔 if it's formatting shouldn't that be a diff pr

Copy link
Member Author

Choose a reason for hiding this comment

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

Also: Do we use the abbreviate Number function anywhere else since it's not removed?

Good question, we use it on the private stats page, so we will keep it 👍

Usually yes but I made other changes and formatting happens on save lol

stories/components/user/UserLink.stories.js
@@ -10,9 +10,8 @@ export const Basic = {
url: "https://www.youtube.com",
clicks: 2,
name: "My youtube channel",
icon: "FaYoutube"
icon: "FaYoutube",
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this also formatting?

Copy link
Member Author

Choose a reason for hiding this comment

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

yes must be

tests/setup/global-setup.js
type: "personal",
bio: `Bio for _test-wcag-user`,
avatar: "https://github.com/eddiejaoude.png",
links: links
}
links: links,
Copy link
Contributor

Choose a reason for hiding this comment

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

Welp, formatting again

Copy link
Member Author

Choose a reason for hiding this comment

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

yep, but I won't let it block this PR 😜

Copy link
Contributor

Choose a reason for hiding this comment

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

Gotchu! 👍🏻

gptkrsh
gptkrsh previously approved these changes Feb 9, 2023
View reviewed changes
Copy link
Contributor

@gptkrsh gptkrsh 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

@dan-mba
Copy link
Member

dan-mba commented Feb 9, 2023

I fixed the test automation to make it pass.

There are 2 images on the index page that show the profile with stats.
Bio Section
Your Links

Other than that it looks good

@eddiejaoude eddiejaoude mentioned this pull request Feb 10, 2023
Merged
7 tasks
@eddiejaoude
Copy link
Member Author

I fixed the test automation to make it pass.

thank you @dan-mba 🔥

There are 2 images on the index page that show the profile with stats.

oh good spot 💯

@eddiejaoude eddiejaoude merged commit 9f33866 into main Feb 10, 2023
@eddiejaoude eddiejaoude deleted the remove-public-stats branch February 10, 2023 00:46
@eddiejaoude eddiejaoude mentioned this pull request Feb 10, 2023
Merged
6 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@github-actions github-actions[bot] github-actions[bot] left review comments

@gptkrsh gptkrsh gptkrsh left review comments

Assignees
No one assigned
Labels
large Pull request with more than 30 changed lines storybook tests
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[FEATURE] Remove public stats
3 participants
@eddiejaoude @dan-mba @gptkrsh