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

feat: auto mask in Flutter #555

Open
wants to merge 16 commits into
base: feat/user-step
Choose a base branch
from

Conversation

ahmedAlaaInstabug
Copy link
Contributor

Description of the change

feat: enhance private views

Type of change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Related issues

Jira iD : MOB-17833

Checklists

Development

  • Lint rules pass locally
  • The code changed/added as part of this pull request has been covered with tests

Code review

  • This pull request has a descriptive title and information useful to a reviewer
  • Issue from task tracker has a link to this pull request

@ahmedAlaaInstabug ahmedAlaaInstabug changed the base branch from refactor/monorepo to feat/user-step February 26, 2025 00:10
@InstabugCI
Copy link
Collaborator

InstabugCI commented Feb 27, 2025

Warnings
⚠️

Coverage for instabug_http_client is less than the expected threshold of 80%

Coverage Report

Label Coverage Status
instabug_dio_interceptor 92.5%
instabug_flutter 82.9%
instabug_flutter_modular 91.2%
instabug_http_client 69.6%

Generated by 🚫 dangerJS against 4bef406

@ahmedAlaaInstabug ahmedAlaaInstabug changed the title feat: enhance private views feat: auto mask in Flutter Mar 5, 2025
@ahmedAlaaInstabug ahmedAlaaInstabug marked this pull request as ready for review March 5, 2025 13:55
@kholood-ea kholood-ea requested a review from AyaMahmoud148 March 6, 2025 09:52
Copy link
Contributor

@AndrewAminInstabug AndrewAminInstabug left a comment

Choose a reason for hiding this comment

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

@ahmedAlaaInstabug Awesome work! 🚀 and great effort 👏🏻👏🏻


// Add the new step to the list
_steps.add(route);
Future<dynamic>.delayed(const Duration(milliseconds: 1000), () {
// If this route is in the array, report it and remove it from the list
if (_steps.contains(route)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

@ahmedAlaaInstabug Won't the condition _steps.contains(route) always evaluate to true since _steps.add(route) is executed beforehand?

bool light = true;
double _scale = 1.0; // Initial scale of the image

TextEditingController _controller2 = TextEditingController();
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you rename and dispose of this controller?

)
: const Center(child: CircularProgressIndicator()),
),
const SizedBox(height: 24),
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need these multiple SizedBox widgets ?

return renderObject.paintBounds.shift(globalOffset);
}

// The is the same implementation used in RenderBox.localToGlobal (a subclass of RenderObject)
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor typo.

Copy link
Contributor

Choose a reason for hiding this comment

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

@ahmedAlaaInstabug Could you mark this class as @internal and adjust its methods to either private or @internal, depending on their calling scope, since it involves highly sensitive logic?

Copy link
Contributor

Choose a reason for hiding this comment

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

Unable to locate unit tests for PrivateViewsManager.I.addAutoMasking and InstabugWidget.

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

Successfully merging this pull request may close these issues.

4 participants