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: Added a new way to redefine how we declare EdgeInsets #276

Merged

Conversation

superiorsd10
Copy link
Contributor

Description

Previously, there was only one way to define MiraiEdgeInsets and it looked like this

              "padding": {
                "top": 8,
                "left": 12,
                "right": 12,
                "bottom": 8
              }

In this pull request, I have added the two new ways as suggested to declare MiraiEdgeInsets.

  1. To give the same padding to all directions, you can now just write like this.
              "padding": 10, 
  1. To give padding to all directions, but in a better and more concise way, you can now just write it like this.
              "padding": [10, 12, 12, 10]

To approach this issue, I have defined a new static method inside MiraiEdgeInsets named _fromJson that takes a dynamic argument and checks if the type of argument is among three acceptable types i.e. num, List<num>, Map<String, dynamic>, and returns MiraiEdgeInsets; otherwise, throw an ArgumentError.

Related Issues

Closes #260

Screen Recording

Screen.Recording.mp4

Type of Change

  • New feature (non-breaking change which adds functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Code refactor
  • Build configuration change
  • Documentation
  • Chore

@divyanshub024 divyanshub024 added this to the v0.7 milestone Feb 29, 2024
@divyanshub024
Copy link
Contributor

Hey @superiorsd10,

Loved what you did with that PR! You nailed the fix and seriously upped our game. Big thanks for your awesome work! 🌟👍

There is one small error I'm getting when using "padding": [10, 12, 10, 10],. Can you please check?

196m┌───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────<…>
flutter: \^[[38;5;196m│ #0   Log.e (package:mirai/src/utils/log.dart:11:42)<…>
flutter: \^[[38;5;196m│ #1   Mirai.fromJson (package:mirai/src/framework/mirai.dart:107:11)<…>
flutter: \^[[38;5;196m├┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄┄<…>
flutter: \^[[38;5;196m│ ⛔ Invalid argument(s): Invalid input format for MiraiEdgeInsets<…>
flutter: \^[[38;5;196m└───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────<…>

@superiorsd10
Copy link
Contributor Author

Hello @divyanshub024 👋

On replicating the behavior stated by you, it's working fine, and I'm not getting any errors. Can you please tell me more about the issue?

@divyanshub024
Copy link
Contributor

Hey @superiorsd10,

I took the pull again and it still showed the same error.

Screen.Recording.2024-02-29.at.5.50.45.PM.1.mov

@superiorsd10
Copy link
Contributor Author

Hello @divyanshub024 👋

I have fixed the above issue. You can pull the latest changes now, and check if it works. I have tested it on my end, and it's working fine.

Thank you,

Copy link
Contributor

@divyanshub024 divyanshub024 left a comment

Choose a reason for hiding this comment

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

LGTM!!
Thank you @superiorsd10 🎉

@divyanshub024 divyanshub024 merged commit a403f12 into Securrency-OSS:main Feb 29, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feat: A new way to redefine how we declareEdgeInsets.
2 participants