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

[Bug]: Notifications - Incorrect text in the notification popup pointing to Settings, but they are not present there #25994

Closed
seaona opened this issue Jul 22, 2024 · 1 comment
Labels
regression-beta-12.0.0 Regression bug that was found in beta in release 12.0.0 release-blocker This bug is blocking the next release Sev2-normal Normal severity; minor loss of service or inconvenience. team-notifications Notifications team type-bug

Comments

@seaona
Copy link
Contributor

seaona commented Jul 22, 2024

Describe the bug

Whenever you try to enable Notifications, you can see how the text points you to Settings>Notifications however, Notifications does not appear in Settings.
If you try to look for it in the Search settings bar, you can see how it seems to appear under Experimental, but it is not present there neither

Expected behavior

Not sure what's the correct behaviour here: either change the miss-leading text and fixed the setting search so it does not appear in settings, or add the Notifications section in Settings

Screenshots/Recordings

Screenshot from 2024-07-22 09-06-28

Screenshot from 2024-07-22 09-06-16

Steps to reproduce

  1. Click Account menu
  2. Click Notifications
  3. See text You can turn off notifications at any time in Settings > Notifications.
  4. Go to Settings
  5. See Notifications is not there in any section
  6. Look for notifications using the search bar
  7. See how it appears in the Experimental section
  8. Click Experimental
  9. See how it's not there

Error messages or log output

No response

Detection stage

In production (default)

Version

12.0.0 beta build

Build type

None

Browser

Chrome

Operating system

Linux

Hardware wallet

No response

Additional context

No response

Severity

No response

@seaona seaona added type-bug Sev2-normal Normal severity; minor loss of service or inconvenience. regression-beta-12.0.0 Regression bug that was found in beta in release 12.0.0 labels Jul 22, 2024
@github-project-automation github-project-automation bot moved this to To be fixed in Bugs by severity Jul 22, 2024
@github-project-automation github-project-automation bot moved this to To be fixed in Bugs by team Jul 22, 2024
@gauthierpetetin gauthierpetetin added team-notifications Notifications team release-blocker This bug is blocking the next release labels Jul 22, 2024
matteoscurati added a commit that referenced this issue Jul 23, 2024
## **Description**

This PR fixes the reported bug:
#25994. The PR
changes the text to inform the user about the option to disable
notifications from the notifications settings page (see images). The
ability to enable/disable notifications directly from the general
settings is expected to be implemented in future releases.

<img width="376" alt="Screenshot 2024-07-22 at 22 37 30"
src="https://github.com/user-attachments/assets/a426fb14-fa2c-45fb-a3d1-bde270df17e8">
<img width="371" alt="Screenshot 2024-07-22 at 22 37 38"
src="https://github.com/user-attachments/assets/b178faf5-0a8f-472a-8971-3519e2932726">


[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/26026?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**

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

## **Pre-merge reviewer checklist**

- [x] I've manually tested the PR (e.g. pull and build branch, run the
app, test code being changed).
- [x] 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.
matteoscurati added a commit that referenced this issue Jul 23, 2024
## **Description**

This PR fixes the reported bug:
#25994. The PR
changes the text to inform the user about the option to disable
notifications from the notifications settings page (see images). The
ability to enable/disable notifications directly from the general
settings is expected to be implemented in future releases.

<img width="376" alt="Screenshot 2024-07-22 at 22 37 30"
src="https://github.com/user-attachments/assets/a426fb14-fa2c-45fb-a3d1-bde270df17e8">
<img width="371" alt="Screenshot 2024-07-22 at 22 37 38"
src="https://github.com/user-attachments/assets/b178faf5-0a8f-472a-8971-3519e2932726">


[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/26026?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**

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

## **Pre-merge reviewer checklist**

- [x] I've manually tested the PR (e.g. pull and build branch, run the
app, test code being changed).
- [x] 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.
chloeYue pushed a commit that referenced this issue Jul 23, 2024
…6042)

…6026)

## **Description**

This PR fixes the reported bug:
#25994. The PR
changes the text to inform the user about the option to disable
notifications from the notifications settings page (see images). The
ability to enable/disable notifications directly from the general
settings is expected to be implemented in future releases.

<img width="376" alt="Screenshot 2024-07-22 at 22 37 30"
src="https://github.com/user-attachments/assets/a426fb14-fa2c-45fb-a3d1-bde270df17e8">
<img width="371" alt="Screenshot 2024-07-22 at 22 37 38"
src="https://github.com/user-attachments/assets/b178faf5-0a8f-472a-8971-3519e2932726">


[![Open in GitHub

Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/26026?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**

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

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

## **Pre-merge reviewer checklist**

- [x] I've manually tested the PR (e.g. pull and build branch, run the
app, test code being changed).
- [x] 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/26042?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**

- [x] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask
Extension Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md).
- [x] 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/develop/.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.
@Prithpal-Sooriya
Copy link
Contributor

This has been merged into v12 and is resolved.

See screenshot The copy has changed and removed the path to settings. We will add this path back in once the settings page has been created.

This settings is still accessible through the notifications page.

Screenshot 2024-07-23 at 15 45 46

@github-project-automation github-project-automation bot moved this from To be fixed to Fixed in Bugs by team Jul 23, 2024
@github-project-automation github-project-automation bot moved this from To be fixed to Fixed in Bugs by severity Jul 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
regression-beta-12.0.0 Regression bug that was found in beta in release 12.0.0 release-blocker This bug is blocking the next release Sev2-normal Normal severity; minor loss of service or inconvenience. team-notifications Notifications team type-bug
Projects
Archived in project
Development

No branches or pull requests

3 participants