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

test github input #1

Open
anyongjitiger opened this issue Aug 15, 2023 · 10 comments
Open

test github input #1

anyongjitiger opened this issue Aug 15, 2023 · 10 comments

Comments

@anyongjitiger
Copy link
Owner

anyongjitiger commented Aug 15, 2023

Proposal

Please re-state the problem that we are trying to solve in this issue.

Hardcoding contact method details URL with a non existent contact method shows magic code verification for a brief amount of time before showing not found page.

What is the root cause of that problem?

The root cause of this problem is on ContactMethodDetailsPage.js file, lines 212-215:

const loginData = this.props.loginList[contactMethod]; if (!contactMethod || !loginData || !loginData.partnerUserID) { return <NotFoundPage />; }

An existent contact method object will have a bunch of properties when created, one of them being partnerUserID which I will use for the solution of the problem. A non existent contact method object will only have the validateCodeSent property set to false, getting it from our resetContactMethodValidateCodeSentState method.

On the above code section, we are only checking if the loginData object exists, and it does, with the validateCodeSent property I mentioned above.

This of course is not the correct check to do to determine if the contact method is found, which leads to showing the Magic Code validation page later on the code, which we show when that condition is true:
!loginData.validatedDate && !isFailedAddContactMethod, and of course, since loginData only has the validateCodeSent it becomes true.

What changes do you think we should make in order to solve the problem?

On lines 212-215 , we should add a check for partnerUserID in the condition:

if (!contactMethod || !loginData || !loginData.partnerUserID) { return <NotFoundPage />; }

https://github.com/Expensify/App/blob/4230f289341c8d6d9a8b780d54da0725d4764b2b/src/libs/actions/User.js#L266-L272to

260231292-5a482dbc-f38f-44f6-b4de-7ff30dac172a.mov

function resetContactMethodValidateCodeSentState(contactMethod) { const existingLoginData = Onyx.get(ONYXKEYS.LOGIN_LIST, {}); if (existingLoginData.hasOwnProperty(contactMethod)) { return; } Onyx.merge(ONYXKEYS.LOGIN_LIST, { [contactMethod]: { validateCodeSent: false, }, }); }

@anyongjitiger
Copy link
Owner Author

function resetContactMethodValidateCodeSentState(contactMethod) { const existingLoginData = Onyx.get(ONYXKEYS.LOGIN_LIST, {}); if (existingLoginData.hasOwnProperty(contactMethod)) { return; } Onyx.merge(ONYXKEYS.LOGIN_LIST, { [contactMethod]: { validateCodeSent: false, }, }); }

@anyongjitiger
Copy link
Owner Author

  function resetContactMethodValidateCodeSentState(contactMethod) {
    const existingLoginData = Onyx.get(ONYXKEYS.LOGIN_LIST, {});
    if (existingLoginData.hasOwnProperty(contactMethod)) {
        return;
    }    Onyx.merge(ONYXKEYS.LOGIN_LIST, {
        [contactMethod]: {
            validateCodeSent: false,
        },
    });
}

@anyongjitiger
Copy link
Owner Author

Proposal

Please re-state the problem that we are trying to solve in this issue.

The problem is that when we click add reaction bubble below a comment. Emoji selector covers LHN

What is the root cause of that problem?

The root cause of the problem is here:

https://github.com/Expensify/App/blob/6319e59c29bf7304654e4ede3c2215a6e65016bf/src/components/EmojiPicker/EmojiPicker.js#L47
Prop anchorOrigin is undefined So it was set to the default value.
https://github.com/Expensify/App/blob/6319e59c29bf7304654e4ede3c2215a6e65016bf/src/components/EmojiPicker/EmojiPicker.js#L14
And the horizontal origin is ‘right’

What changes do you think we should make in order to solve the problem?

We can pass in the anchorOrigin value with a horizontal origin of 'center'.
, or simply set the default horizontal origin to ‘center'

What alternative solutions did you explore? (Optional)

None

Result

截屏2023-08-18 20 08 26

@anyongjitiger
Copy link
Owner Author

Proposal

Please re-state the problem that we are trying to solve in this issue.

The problem is that Money in wallet badge is not center, bottom is bigger than top.

What is the root cause of that problem?

The root cause of the problem is :
The line-height of the badgeText is smaller (16px) than the height of the badge (20px).

What changes do you think we should make in order to solve the problem?

We can set the line-height of the badgeText and the height of the badge to be the same, which will center it.

What alternative solutions did you explore? (Optional)

None

@anyongjitiger
Copy link
Owner Author

Proposal

Please re-state the problem that we are trying to solve in this issue.

if tooltip was in an opened state and the user navigated to external site and came back, tooltip will show again

What is the root cause of that problem?

When the "Add bank account" button is clicked, it opens a Plaid page in an iframe. When the mouse moves within the iframe, the parent page does not receive any mouse events. Once the user closes the iframe and stops moving the mouse, the onMouseEnter function triggers automatically and sets setIsHovered to true, regardless of any other conditions.
https://github.com/Expensify/App/blob/e426b391e83eabe0c4fbf0cc5cbc4cb5fc19b32c/src/components/Hoverable/index.js#L115C1-L117C1

What changes do you think we should make in order to solve the problem?

We can add an event listener to identify if the mouse has entered the iframe:

           this.setIsMouseEnterIframe(true);
     });```

Add if block in the onMouseEnter that checks for the isMouseEnterIframe state
```onMouseEnter: (el) => {
                if (this.state.isMouseEnterIframe) {
                    return;
                }
                this.setIsHovered(true);```

### What alternative solutions did you explore? (Optional)

replaced  
this.setIsHovered(true); 
To
this.setIsHovered(el.target === el.currentTarget)
But this will cause some other tooltips not to show up, such as FAB.

@anyongjitiger
Copy link
Owner Author

Proposal

Please re-state the problem that we are trying to solve in this issue.

if tooltip was in an opened state and the user navigated to external site and came back, tooltip will show again

What is the root cause of that problem?

When the "Add bank account" button is clicked, it opens a Plaid page in an iframe. When the mouse moves within the iframe, the parent page does not receive any mouse events. Once the user closes the iframe and stops moving the mouse, the onMouseEnter function triggers automatically and sets setIsHovered to true, regardless of any other conditions.

https://github.com/Expensify/App/blob/e426b391e83eabe0c4fbf0cc5cbc4cb5fc19b32c/src/components/Hoverable/index.js#L115-L117

What changes do you think we should make in order to solve the problem?

We can add an event listener to identify if the mouse has entered the iframe:

iframe.addEventListener('mouseenter', function () {
           this.setIsMouseEnterIframe(true);
     });

Add if block in the onMouseEnter that checks for the isMouseEnterIframe state:

onMouseEnter: (el) => {
                if (this.state.isMouseEnterIframe) {
                    return;
                }
                this.setIsHovered(true);

What alternative solutions did you explore? (Optional)

replaced
this.setIsHovered(true); 
To
this.setIsHovered(el.target === el.currentTarget)
But this will cause some other tooltips not to show up, such as FAB.

1 similar comment
@anyongjitiger
Copy link
Owner Author

Proposal

Please re-state the problem that we are trying to solve in this issue.

if tooltip was in an opened state and the user navigated to external site and came back, tooltip will show again

What is the root cause of that problem?

When the "Add bank account" button is clicked, it opens a Plaid page in an iframe. When the mouse moves within the iframe, the parent page does not receive any mouse events. Once the user closes the iframe and stops moving the mouse, the onMouseEnter function triggers automatically and sets setIsHovered to true, regardless of any other conditions.

https://github.com/Expensify/App/blob/e426b391e83eabe0c4fbf0cc5cbc4cb5fc19b32c/src/components/Hoverable/index.js#L115-L117

What changes do you think we should make in order to solve the problem?

We can add an event listener to identify if the mouse has entered the iframe:

iframe.addEventListener('mouseenter', function () {
           this.setIsMouseEnterIframe(true);
     });

Add if block in the onMouseEnter that checks for the isMouseEnterIframe state:

onMouseEnter: (el) => {
                if (this.state.isMouseEnterIframe) {
                    return;
                }
                this.setIsHovered(true);

What alternative solutions did you explore? (Optional)

replaced
this.setIsHovered(true); 
To
this.setIsHovered(el.target === el.currentTarget)
But this will cause some other tooltips not to show up, such as FAB.

@anyongjitiger
Copy link
Owner Author

Add a mutation observer in componentDidMount that checks if there is an iFrame.

    componentDidMount() {
        document.addEventListener('visibilitychange', this.handleVisibilityChange);
        const self = this;
        this.observer = new MutationObserver(function (mutations) {
            mutations.forEach(function (mutation) {
                mutation.addedNodes.forEach(function (node) {
                    if (node.tagName === 'IFRAME') {
                        node.addEventListener('mouseenter', function () {
                            self.setIsMouseEnterIframe(true);
                        });
                    }
                });
            });
        });
        this.observer.observe(document.body, {childList: true, subtree: false});
    }

Disconnect the mutation observer in componentWillUnmount:

componentWillUnmount() {
        document.removeEventListener('visibilitychange', this.handleVisibilityChange);
        if (this.observer) {
            this.observer.disconnect();
        }
    }

Check if state.isMouseEnterIframe is true in onMouseEnter:

onMouseEnter: (el) => {
                if (state.isMouseEnterIframe) {
                    this.setIsMouseEnterIframe(false);
                    return;
                }
                this.setIsHovered(true);

@anyongjitiger
Copy link
Owner Author

Proposal

Please re-state the problem that we are trying to solve in this issue.

Avatar of a new user changes to another default avatar when switching from offline to online

What is the root cause of that problem?

When we are offline and create a chat with a new user, we use an optimistic ID as their account ID:
截屏2023-10-11 14 10 01

When switching from offline to online, the server will update the optimistic ID to use the correct ID instead. At the same time, it also changed the address of the avatar:
截屏2023-10-11 15 20 00

https://d2k5ns\2zxldvw.cloudfront.net/images/avatars/default-avatar_7.png

We're using function getAvatar to calculate the avatar here:

function getAvatar(avatarURL: string, accountID: number): React.FC<SvgProps> | string {
    return isDefaultAvatar(avatarURL) ? getDefaultAvatar(accountID) : avatarURL;
}

When a new avatar address is passed into the isDefaultAvatar function, it returns true. As a result, we recalculated the avatar address with the new accountID, and it return a different avatar.

What changes do you think we should make in order to solve the problem?

We can change the logic in isDefaultAvatar, remove this part:
avatarURL.includes('images/avatars/default-avatar_')

if (avatarURL.includes('images/avatars/avatar_')  || avatarURL.includes('images/avatars/user/default')) {
            return true;
        }

What alternative solutions did you explore? (Optional)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant