Skip to content
This repository was archived by the owner on Jun 7, 2020. It is now read-only.

[BUG] Hide login button when setting is disabled#2049

Merged
rafaelks merged 5 commits intodevelopfrom
chore/do_not_show_email_option_when_disabled
Jul 20, 2018
Merged

[BUG] Hide login button when setting is disabled#2049
rafaelks merged 5 commits intodevelopfrom
chore/do_not_show_email_option_when_disabled

Conversation

@rafaelks
Copy link
Copy Markdown
Contributor

@RocketChat/ios

@rafaelks rafaelks added this to the 3.0.2 milestone Jul 20, 2018
@rafaelks rafaelks requested a review from filipealva July 20, 2018 14:20
emailAuthRow.loginButton.isHidden = !settings.isUsernameEmailAuthenticationEnabled

if !settings.isUsernameEmailAuthenticationEnabled {
emailAuthRow.registerButton.isHidden = true
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@rafaelks we could put the new line inside the if block. That way we would evaluate settings.isUsernameAuthenticationEnabled just once.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

👍

@filipealva
Copy link
Copy Markdown
Contributor

filipealva commented Jul 20, 2018

@rafaelks actually if isUsernameAuthenticationEnabled is false we shouldn't even show the emailAuthRow and neither the separator of the login services section

@rafaelks
Copy link
Copy Markdown
Contributor Author

@filipealva Done!

@filipealva
Copy link
Copy Markdown
Contributor

@rafaelks Nice! but just hiding the cell still let us considering its height on uitableview's content size, which means that if you have a server with a certain number of login services you'll have an issue with an extra offset

@rafaelks
Copy link
Copy Markdown
Contributor Author

@filipealva I see... this is not using AutoLayout, so when the registerButton was hidden it was already having a white space. Will make some adjustments here.

@codecov
Copy link
Copy Markdown

codecov Bot commented Jul 20, 2018

Codecov Report

Merging #2049 into develop will increase coverage by 0.5%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           develop    #2049     +/-   ##
==========================================
+ Coverage    35.74%   36.24%   +0.5%     
==========================================
  Files          374      374             
  Lines        17432    17428      -4     
==========================================
+ Hits          6231     6317     +86     
+ Misses       11201    11111     -90
Impacted Files Coverage Δ
...hat/Controllers/Auth/AuthTableViewController.swift 0% <0%> (ø) ⬆️
...at/Controllers/Base/BaseNavigationController.swift 42.5% <0%> (-12.5%) ⬇️
...at/Controllers/Auth/AuthNavigationController.swift 32.2% <0%> (-10.17%) ⬇️
...Views/Cells/Subscription/ChatPreviewModeView.swift 43.47% <0%> (-8.7%) ⬇️
...ket.Chat/Controllers/Base/BaseViewController.swift 79.48% <0%> (-5.13%) ⬇️
Rocket.Chat/Managers/AppManager.swift 62.61% <0%> (-0.47%) ⬇️
...Controllers/Auth/ConnectServerViewController.swift 64.03% <0%> (ø) ⬆️
...hat/Controllers/Base/MainSplitViewController.swift 86.04% <0%> (ø) ⬆️
...lers/Notification/NotificationViewController.swift 52.22% <0%> (ø) ⬆️
Rocket.Chat/Theme/ThemeableViews.swift 83.22% <0%> (+0.62%) ⬆️
... and 8 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1ec08e8...a5a8d0a. Read the comment docs.

Copy link
Copy Markdown
Contributor

@filipealva filipealva left a comment

Choose a reason for hiding this comment

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

👍

@rafaelks rafaelks merged commit 856f2bb into develop Jul 20, 2018
@rafaelks rafaelks deleted the chore/do_not_show_email_option_when_disabled branch July 20, 2018 18:33
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants