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.

Sign up for GitHub

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

Jump to bottom

[AP-121]-Friend req count #554

Merged
merged 8 commits into from
Dec 10, 2021
Merged

[AP-121]-Friend req count #554

merged 8 commits into from
Dec 10, 2021

Conversation

KemoPaw
Copy link
Contributor

@KemoPaw KemoPaw commented Dec 7, 2021

What this PR does πŸ“–

https://satellite-im.atlassian.net/browse/AP-121

Which issue(s) this PR fixes πŸ”¨

Fixes #

Special notes for reviewers πŸ—’οΈ

Additional comments 🎀

stavares843
stavares843 requested changes Dec 7, 2021
View reviewed changes
Copy link
Member

@stavares843 stavares843 left a comment

Choose a reason for hiding this comment

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

neat πŸŽ‰

any suggestions on how to avoid 2 digits to be overlapping the copy of the request? maybe 9+ instead? we can ask on the channel as well

Captura de ecrã 2021-12-07, às 23 41 39

@stavares843 stavares843 added Missing Dev Review A Dev and a Dev Approver need to review the PR, then mark as Ready for QA Changes Requested Changes have been requested by Dev or QA, once the changes have been made, remove this label. labels Dec 7, 2021
@InfamousVague
Copy link

We should add some padding right to the button when there are notifications to avoid the overlap

josephmcg
josephmcg suggested changes Dec 8, 2021
View reviewed changes
components/tailored/friends/nav/Nav.less Outdated Show resolved Hide resolved
components/tailored/friends/nav/Nav.html Outdated Show resolved Hide resolved
components/tailored/friends/nav/Nav.html Outdated Show resolved Hide resolved
components/tailored/friends/nav/Nav.html Outdated Show resolved Hide resolved
components/tailored/friends/nav/Nav.html Outdated Show resolved Hide resolved
components/tailored/friends/nav/Nav.vue Show resolved Hide resolved
components/tailored/friends/nav/Nav.vue Outdated Show resolved Hide resolved
@stavares843 stavares843 added missing fixing conflict Conflict needs to be handled, then re-tested by devs/qa and removed Missing Dev Review A Dev and a Dev Approver need to review the PR, then mark as Ready for QA labels Dec 8, 2021
@WanderingHogan
Copy link
Contributor

/rebase

2 similar comments
@WanderingHogan
Copy link
Contributor

/rebase

@stavares843
Copy link
Member

/rebase

@stavares843 stavares843 added Missing Dev Review A Dev and a Dev Approver need to review the PR, then mark as Ready for QA and removed Changes Requested Changes have been requested by Dev or QA, once the changes have been made, remove this label. labels Dec 9, 2021
josephmcg
josephmcg reviewed Dec 9, 2021
View reviewed changes
Copy link
Contributor

@josephmcg josephmcg left a comment

Choose a reason for hiding this comment

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

a couple minor things, mostly removing dead code

components/tailored/friends/nav/Nav.less Outdated Show resolved Hide resolved
components/tailored/friends/nav/Nav.vue Outdated Show resolved Hide resolved
components/tailored/friends/nav/Nav.html Outdated Show resolved Hide resolved
components/tailored/friends/nav/Nav.less Outdated Show resolved Hide resolved
@stavares843 stavares843 added Changes Requested Changes have been requested by Dev or QA, once the changes have been made, remove this label. and removed Missing Dev Review A Dev and a Dev Approver need to review the PR, then mark as Ready for QA labels Dec 9, 2021
@KemoPaw
Copy link
Contributor Author

KemoPaw commented Dec 9, 2021

@josephmcg Changes have been made, thank you for the notes/feedback!

@stavares843 stavares843 removed the missing fixing conflict Conflict needs to be handled, then re-tested by devs/qa label Dec 9, 2021
@stavares843 stavares843 added Missing Dev Review A Dev and a Dev Approver need to review the PR, then mark as Ready for QA missing fixing conflict Conflict needs to be handled, then re-tested by devs/qa and removed Changes Requested Changes have been requested by Dev or QA, once the changes have been made, remove this label. labels Dec 9, 2021
josephmcg
josephmcg approved these changes Dec 10, 2021
View reviewed changes
Copy link
Contributor

@josephmcg josephmcg left a comment

Choose a reason for hiding this comment

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

πŸš€

@josephmcg josephmcg removed the Missing Dev Review A Dev and a Dev Approver need to review the PR, then mark as Ready for QA label Dec 10, 2021
@stavares843 stavares843 merged commit 9039e8d into dev Dec 10, 2021
@stavares843 stavares843 deleted the FriendReqCount branch December 10, 2021 00:57
@stavares843 stavares843 removed the missing fixing conflict Conflict needs to be handled, then re-tested by devs/qa label Dec 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@InfamousVague InfamousVague InfamousVague approved these changes

@WanderingHogan WanderingHogan WanderingHogan approved these changes

@stavares843 stavares843 stavares843 approved these changes

@josephmcg josephmcg josephmcg approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@KemoPaw @InfamousVague @WanderingHogan @stavares843 @josephmcg