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

[Bug]: Token detection prompt not dismissing #29059

Closed
bergeron opened this issue Dec 10, 2024 · 1 comment · Fixed by #29061
Closed

[Bug]: Token detection prompt not dismissing #29059

bergeron opened this issue Dec 10, 2024 · 1 comment · Fixed by #29061
Labels
regression-prod-12.9.0 Regression bug that was found in production in release 12.9.0 release-12.10.1 Issue or pull request that will be included in release 12.10.1 type-bug

Comments

@bergeron
Copy link
Contributor

Describe the bug

In 12.9.0, the token detection prompt does not dismiss like it did in previous versions. It used to dismiss regardless of how many tokens you chose to import. Now if you don't import them all, it stays around suggesting the remaining tokens you did not choose to import.

Not sure if this was intentional. Compare with 12.8 behavior.

Expected behavior

No response

Screenshots/Recordings

No response

Steps to reproduce

Load a new wallet with erc20 tokens.
It should autodetect them.
Only select a subset to import.
The autodetect prompt remains, but maybe should not.

Error messages or log output



Detection stage


In production (default)


Version


12.9.0


Build type


None


Browser


Chrome


Operating system


MacOS


Hardware wallet


No response


Additional context


No response


Severity


No response

      		
    

      
  





        



            

              
            

        

      


      
    








      

    



      

  
            

    

      
    

    


      

          @bergeron
bergeron




            added
      

  regression-prod-12.9.0

  Regression bug that was found in production in release 12.9.0

      

  type-bug


  labels


      Dec 10, 2024

    

  



    


    

    

      
    

    


      

          @github-project-automation
github-project-automation
bot



          moved this to To be fixed
  in Bugs by team


      Dec 10, 2024

    

  



    

    

      
    

    


      

          @github-project-automation
github-project-automation
bot



          moved this to To be fixed
  in Bugs by severity


      Dec 10, 2024

    

  



  

  

    
  


  

      

    @salimtb
salimtb



    mentioned this issue
    
      Dec 10, 2024
    





  


  

      
   Merged

  





  
  

      
        
      
      

  
    
    
  


    7 tasks
  





  




  


  
    
  
  

    
  github-merge-queue bot

      pushed a commit
      that referenced
      this issue

    
      Dec 10, 2024
    
    

      


  

    

      


  

      
        @salimtb
      
        @bergeron
  





      

        
          fix: fix sticky autodetection banner (#29061)
        

          
            
          

      


      

          

    
    
    

  

    


      


      

          

    
    
    

  

  

    
  



      


      
      

        
          5789e1c
        
      

    

  

    

      
<!--
Please submit this PR as a draft initially.
Do not mark it as "Ready for review" until the template has been
completely filled out, and PR status checks have passed at least once.
-->

## **Description**

fix sticky autodetection banner
<!--
Write a short description of the changes included in this pull request,
also include relevant motivation and context. Have in mind the following
questions:
1. What is the reason for the change?
2. What is the improvement/solution?
-->

[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/29061?quickstart=1)

## **Related issues**

Fixes: #29059 

## **Manual testing steps**

1. Go to autodetected banner
2. click on ignore all
3. you should have allNetworks selected

## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**

<!-- [screenshots/recordings] -->

### **After**






https://drive.google.com/file/d/11zcmgV1Bdq5wx9MkDlhxr8oDBr9DGB3d/view?usp=sharing




<!-- [screenshots/recordings] -->

## **Pre-merge author checklist**

- [ ] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask
Extension Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [ ] I've completed the PR template to the best of my ability
- [ ] I’ve included tests if applicable
- [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [ ] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.

## **Pre-merge reviewer checklist**

- [ ] I've manually tested the PR (e.g. pull and build branch, run the
app, test code being changed).
- [ ] I confirm that this PR addresses all acceptance criteria described
in the ticket it closes and includes the necessary testing evidence such
as recordings and or screenshots.

---------

Co-authored-by: Brian Bergeron <[email protected]>

    




    

  




    

    


    

    

      
    

    


      

          @github-project-automation
github-project-automation
bot



          moved this from To be fixed
  to Fixed
  in Bugs by severity


      Dec 10, 2024

    

  



    

    

      
    

    


      

          @github-project-automation
github-project-automation
bot



          moved this from To be fixed
  to Fixed
  in Bugs by team


      Dec 10, 2024

    

  



    

    

      
    

    


      

          @metamaskbot
metamaskbot




          added
  the 

  release-12.10.1

  Issue or pull request that will be included in release 12.10.1
 label


      Dec 10, 2024

    

  



  


  
    
  
  

    
  gambinish 

      pushed a commit
      that referenced
      this issue

    
      Dec 11, 2024
    
    

      


  

    

      


  

      
        @salimtb
      
        @bergeron
      
        @gambinish
  





      

        
          fix: fix sticky autodetection banner (#29061)
        

          
            
          

      


      

          

    
    
    

  

      


      

          

    
    
    

  

  

    
  



      


      
      

        
          a948d7a
        
      

    

  

    

      
<!--
Please submit this PR as a draft initially.
Do not mark it as "Ready for review" until the template has been
completely filled out, and PR status checks have passed at least once.
-->

## **Description**

fix sticky autodetection banner
<!--
Write a short description of the changes included in this pull request,
also include relevant motivation and context. Have in mind the following
questions:
1. What is the reason for the change?
2. What is the improvement/solution?
-->

[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/29061?quickstart=1)

## **Related issues**

Fixes: #29059 

## **Manual testing steps**

1. Go to autodetected banner
2. click on ignore all
3. you should have allNetworks selected

## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**

<!-- [screenshots/recordings] -->

### **After**






https://drive.google.com/file/d/11zcmgV1Bdq5wx9MkDlhxr8oDBr9DGB3d/view?usp=sharing




<!-- [screenshots/recordings] -->

## **Pre-merge author checklist**

- [ ] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask
Extension Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [ ] I've completed the PR template to the best of my ability
- [ ] I’ve included tests if applicable
- [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [ ] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.

## **Pre-merge reviewer checklist**

- [ ] I've manually tested the PR (e.g. pull and build branch, run the
app, test code being changed).
- [ ] I confirm that this PR addresses all acceptance criteria described
in the ticket it closes and includes the necessary testing evidence such
as recordings and or screenshots.

---------

Co-authored-by: Brian Bergeron <[email protected]>

    




    

  




  

  

    
  


  

      

    @gambinish
gambinish



    mentioned this issue
    
      Dec 11, 2024
    





  


  

      
   Merged

  





  
  

      
        
      
      

  
    
    
  


    14 tasks
  





  




  


  
    
  
  

    
  danjm 

      pushed a commit
      that referenced
      this issue

    
      Dec 11, 2024
    
    

      


  

    

      


  

      
        @gambinish
      
        @salimtb
      
        @bergeron
  





      

        
          fix: [cherry-pick] fix sticky autodetection banner (#29061) (#29078)
        

          
            
          

      


      

          

    
    
    

  

    


      


      

          

    
    
    

  

  

    
  



      


      
      

        
          3fc8b20
        
      

    

  

    

      
cherry-picks
5789e1c
(#29061)

<!--
Please submit this PR as a draft initially.
Do not mark it as "Ready for review" until the template has been
completely filled out, and PR status checks have passed at least once.
-->

## **Description**

fix sticky autodetection banner
<!--
Write a short description of the changes included in this pull request,
also include relevant motivation and context. Have in mind the following
questions:
1. What is the reason for the change?
2. What is the improvement/solution? -->

[![Open in GitHub

Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/29061?quickstart=1)

## **Related issues**

Fixes: #29059 

## **Manual testing steps**

1. Go to autodetected banner
2. click on ignore all
3. you should have allNetworks selected

## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**

<!-- [screenshots/recordings] -->

### **After**







https://drive.google.com/file/d/11zcmgV1Bdq5wx9MkDlhxr8oDBr9DGB3d/view?usp=sharing




<!-- [screenshots/recordings] -->

## **Pre-merge author checklist**

- [ ] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask
Extension Coding

Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [ ] I've completed the PR template to the best of my ability
- [ ] I’ve included tests if applicable
- [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [ ] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.

## **Pre-merge reviewer checklist**

- [ ] I've manually tested the PR (e.g. pull and build branch, run the
app, test code being changed).
- [ ] I confirm that this PR addresses all acceptance criteria described
in the ticket it closes and includes the necessary testing evidence such
as recordings and or screenshots.

---------


<!--
Please submit this PR as a draft initially.
Do not mark it as "Ready for review" until the template has been
completely filled out, and PR status checks have passed at least once.
-->

## **Description**

<!--
Write a short description of the changes included in this pull request,
also include relevant motivation and context. Have in mind the following
questions:
1. What is the reason for the change?
2. What is the improvement/solution?
-->

[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/29078?quickstart=1)

## **Related issues**

Fixes:

## **Manual testing steps**

1. Go to this page...
2.
3.

## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**

<!-- [screenshots/recordings] -->

### **After**

<!-- [screenshots/recordings] -->

## **Pre-merge author checklist**

- [ ] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask
Extension Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [ ] I've completed the PR template to the best of my ability
- [ ] I’ve included tests if applicable
- [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [ ] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.

## **Pre-merge reviewer checklist**

- [ ] I've manually tested the PR (e.g. pull and build branch, run the
app, test code being changed).
- [ ] I confirm that this PR addresses all acceptance criteria described
in the ticket it closes and includes the necessary testing evidence such
as recordings and or screenshots.

Co-authored-by: Salim TOUBAL <[email protected]>
Co-authored-by: Brian Bergeron <[email protected]>

    




    

  









      

  
      



  

  @smoovhogg





  


    

      

  

    

        
    
    

  


      
          
  
      
            Copy link

  

      
    

  


  

      



      

  


  

    

  





      


        


  
    

      
          


Describe the bug


In 12.9.0, the token detection prompt does not dismiss like it did in previous versions.  It used to dismiss regardless of how many tokens you chose to import. Now if you don't import them all, it stays around suggesting the remaining tokens you did not choose to import.


Not sure if this was intentional.  Compare with 12.8 behavior.


Expected behavior


No response


Screenshots/Recordings


No response


Steps to reproduce


Load a new wallet with erc20 tokens.

It should autodetect them.

Only select a subset to import.

The autodetect prompt remains, but maybe should not.


Error messages or log output




Detection stage


In production (default)


Version


12.9.0


Build type


None


Browser


Chrome


Operating system


MacOS


Hardware wallet


No response


Additional context


No response


Severity


No response



      		
    

  





        


            

            

              
            

        

      


      

            
  

    
  
    
    

  

  



    












      

  
            











  
  

    

      

  





  




  
  

              

  
    Sign up for free
    to join this conversation on GitHub.
    Already have an account?
    Sign in to comment


  



  






  
                  




    

  


      
  

    Assignees
  



        

    No one assigned







      


  


  

    Labels
  



    

      

    regression-prod-12.9.0

  Regression bug that was found in production in release 12.9.0
  

    release-12.10.1

  Issue or pull request that will be included in release 12.10.1
  

    type-bug









      

  

    

        

    Projects
  


        




        
  

    

      
          
        Bugs by severity
        
    

    

      

          

  

    
      Archived in project



      

    

    

  







  



      


  

    
  

    Milestone
  


      No milestone





    
      
          


  

    
      

        
    

    Development
  




          
  
Successfully merging a pull request may close this issue.





    


  
          
      fix: fix sticky autodetection banner

    
      MetaMask/metamask-extension
    





    
  




      

    

  
      

  

    

      3 participants
    

    

        
          @bergeron 
        
          @metamaskbot 
        
          @smoovhogg