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

Supported interface orientations fix #399

Merged

Conversation

moheny
Copy link

@moheny moheny commented Jun 17, 2018

This fixes #398

Copy link
Owner

@skywinder skywinder left a comment

Choose a reason for hiding this comment

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

Thank you very much for supporting the project! let's make small fixes, that I point out above in the code and I will release the new version! 👍

@@ -91,7 +92,9 @@ - (UIWindow *)window
UIWindow *window = [[UIWindow alloc] initWithFrame:[UIScreen mainScreen].bounds];
window.windowLevel = self.windowLevel;
window.backgroundColor = [UIColor clearColor];
window.rootViewController = [SWActionSheetVC new];
SWActionSheetVC *new = [SWActionSheetVC new];
Copy link
Owner

Choose a reason for hiding this comment

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

new is reserved name, sot lets use more related to this class name.

@@ -502,6 +506,7 @@
ALWAYS_SEARCH_USER_PATHS = NO;
CLANG_ENABLE_OBJC_ARC = YES;
COPY_PHASE_STRIP = YES;
DEVELOPMENT_TEAM = M8UU7YPNPZ;
Copy link
Owner

Choose a reason for hiding this comment

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

please remove dev team from this PR

Copy link
Author

Choose a reason for hiding this comment

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

Sorry for that.

- (BOOL)shouldAutorotate
{
return YES;
if (self.maskVC == UIInterfaceOrientationPortrait){
Copy link
Owner

Choose a reason for hiding this comment

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

it could be realised easily

Copy link
Author

Choose a reason for hiding this comment

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

What do you mean? Can you please elaborate or suggest a better approach?

Copy link
Owner

@skywinder skywinder Oct 13, 2018

Choose a reason for hiding this comment

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

Here is is:
return (!self.maskVC == UIInterfaceOrientationPortrait);

:)

Copy link
Owner

Choose a reason for hiding this comment

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

Please, put it this way, so it makes the style more readable. it's just about good code style

@moheny
Copy link
Author

moheny commented Sep 5, 2018

Hey can you please revisit?

@skywinder
Copy link
Owner

Dear @moheny , thank you very much for the stay in touch.
I focused on other projects and doesn't work on this project last year.
I'm coming back to coding pretty soon. I will test it and release an update pretty soon.

Cheers 👍

@skywinder
Copy link
Owner

@moheny
fyi: #348

I would be glad to build a team of contributors.
If you are actively using it this library - I would be glad to make you collaborator as well.

@nimomix
Copy link

nimomix commented Nov 7, 2018

Looks good, when is this expected to be merged ?

@yoavkamary
Copy link

@skywinder is this going to be merged anytime soon? Thanks

@skywinder
Copy link
Owner

Hello everyone! Thank you for your patience! I'm back and will cleanup everything. Thank you very much for your contribution!
I need help with future support. If anyone there is interested - please drop a comment here, please!
#348

@@ -13,7 +13,9 @@

- (void)showFromBarButtonItem:(UIBarButtonItem *)item animated:(BOOL)animated;

- (instancetype)initWithView:(UIView *)view windowLevel:(UIWindowLevel)windowLevel;
- (instancetype)initWithView:(UIView *)view windowLevel:(UIWindowLevel)windowLevel withSupportedOrientation:(UIInterfaceOrientationMask) mask;
Copy link
Owner

Choose a reason for hiding this comment

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

please, do not change already existing init methind, otherwise we are loosing backward compatability. we have to make new one and make an alias to this.

- (BOOL)shouldAutorotate
{
return YES;
if (self.maskVC == UIInterfaceOrientationPortrait){
Copy link
Owner

Choose a reason for hiding this comment

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

Please, put it this way, so it makes the style more readable. it's just about good code style

@skywinder
Copy link
Owner

@moheny I added to comments, please, fix it or allow me to change this branch. ANd I will merge it soon! https://help.github.com/en/github/collaborating-with-issues-and-pull-requests/allowing-changes-to-a-pull-request-branch-created-from-a-fork

@@ -502,6 +503,7 @@
ALWAYS_SEARCH_USER_PATHS = NO;
CLANG_ENABLE_OBJC_ARC = YES;
COPY_PHASE_STRIP = YES;
DEVELOPMENT_TEAM = "";
Copy link
Owner

Choose a reason for hiding this comment

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

Please, remove not related to this fix changes

} else {
return YES;
}

Copy link
Owner

Choose a reason for hiding this comment

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

more details about htis here:

https://wiki.c2.com/?ReturnBooleanEvaluations

This is a common idiom of just about every language, including Java.

Many programmers write methods that look like the following:

  public boolean isBigger() {
  // return if a is bigger than b
  if (a > b)
	return true;
  else
	return false;
  }


The if statement is redundant. Boolean expressions (a > b) evaluate to a boolean value -- if your method needs to return it, then there's no need to have an if statement. The following code is equivalent to the previous:

  public boolean isBigger () {
	// return if a is bigger than b
	return (a > b);
  }

@skywinder skywinder merged commit b23f784 into skywinder:develop Dec 10, 2019
@skywinder skywinder mentioned this pull request Feb 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SupportedInterfaceOrientations Horizontal fixed
4 participants