-
Notifications
You must be signed in to change notification settings - Fork 10.4k
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
fix: security tab visibility #31996
fix: security tab visibility #31996
Conversation
Signed-off-by: Abhinav Kumar <[email protected]>
Signed-off-by: Abhinav Kumar <[email protected]>
Signed-off-by: Abhinav Kumar <[email protected]>
🦋 Changeset detectedLatest commit: 0345ad8 The changes in this PR will be included in the next version bump. This PR includes changesets to release 32 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #31996 +/- ##
===========================================
+ Coverage 55.06% 55.12% +0.05%
===========================================
Files 2309 2307 -2
Lines 50977 50923 -54
Branches 10430 10424 -6
===========================================
Hits 28070 28070
+ Misses 20386 20351 -35
+ Partials 2521 2502 -19
Flags with carried forward coverage won't be shown. Click here to find out more. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we add a test to ensure the behavior?
Signed-off-by: Abhinav Kumar <[email protected]>
Co-authored-by: Debdut Chakraborty <[email protected]>
Signed-off-by: Abhinav Kumar <[email protected]>
apps/meteor/client/views/account/security/AccountSecurityPage.tsx
Outdated
Show resolved
Hide resolved
Co-authored-by: Douglas Fabris <[email protected]>
@abhinavkrin only accepting my suggestion will break the code, pay attention 🚨 |
Signed-off-by: Abhinav Kumar <[email protected]>
Signed-off-by: Abhinav Kumar <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All good at frontend perspective
Co-authored-by: Marcos Spessatto Defendi <[email protected]>
Signed-off-by: Abhinav Kumar <[email protected]>
Signed-off-by: Abhinav Kumar <[email protected]>
cfb42dc
to
88e8acc
Compare
Looks like this PR is ready to merge! 🎉 |
Looks like this PR is not ready to merge, because of the following issues:
|
Proposed changes (including videos or screenshots)
This change fixes the visibility of the Security tab in user settings, ensuring that users can change their passwords even when 2FA or E2E is disabled. Previously, the option to change the password was hidden unless one of these features was enabled, which could lead to user confusion. This fix ensures that the Security tab is always visible, but it only displays relevant settings based on the user's configuration.
Screencast.from.2024-03-14.20-49-43.mp4
Issue(s)
This PR addresses the issue where users were unable to find the password change option due to the conditional visibility of the Security tab based on 2FA or E2E settings.
Steps to test or reproduce
Further comments
SUP-458