Customize errorMessage position #271

ghost · 2019-01-29T18:19:06Z

Error messages can be set above or below textfield

Allow for error label to be displayed below the text field.

jaredegan

Looks good to me 👌

jaredegan · 2019-02-15T17:00:42Z

What do you think @k0nserv?

jaredegan · 2019-02-15T17:16:06Z

Actually, the new public/open vars are missing documentation blocks as comments.

jaredegan · 2019-02-18T18:55:00Z

The textColor of errorLabel does not get updated in updateColors.

jaredegan · 2019-02-18T18:58:29Z

Sources/SkyFloatingLabelTextField.swift

        } else {
            textAlignment = .right
            titleLabel.textAlignment = .right
+            titleLabel.textAlignment = .right


Copy paste error, you meant to paste in errorLabel here.

spencerdiniz · 2019-02-18T19:01:30Z

The textColor of errorLabel does not get updated in updateColors.

Is this necessary?
errorLabel.textColor is always errorColor, which is set in createErrorLabel().

spencerdiniz · 2019-02-18T19:02:03Z

Issues addressed and branch updated.

jaredegan · 2019-02-18T19:06:54Z

It is necessary. If a user of this class changes the errorColor after initialization, the color of errorLabel will not be updated and it will still be red.

spencerdiniz · 2019-02-18T19:14:22Z

It is necessary. If a user of this class changes the errorColor after initialization, the color of errorLabel will not be updated and it will still be red.

Fixed and pushed.

…dded unit tests, adjust syntax to be consistent with project

Assorted adjustments to better fit SkyFloatingLabelTextField style

k0nserv · 2019-02-21T11:14:59Z

Hey 👋 I am a bit concerned about feature creep. There's more and more PRs that add custom variations and I am concerned that all these variants will decrease maintainability and increase the risk of introducing bugs. Because of this I'd ask if we can consider whether this can be implemented as a subclass rather than a variant

spencerdiniz · 2019-02-21T12:31:41Z

Hi. We did consider implementing this as a subclass, but that would require us to refactor a lot of our existing code that uses the current class. That’s why we decided to include the feature in the class itself and not subclass.

Also, when we considered adding this feature through subclassing, we found out the some of the properties and methods are private, which doesn't allow us to override them.

Bernix01 · 2019-04-02T20:05:31Z

any update on this?

odanu · 2019-04-07T19:08:55Z

Yeap, indeed guys, are these commits going to join the main branch? :)

ghost · 2019-05-02T20:40:24Z

@k0nserv and @jaredegan can we approve this PR to master? I understand the manutenability concern but the unit tests can help us to keep code working. Besides that, this specific feature is not that complex, it is about the error message position.

jaredegan · 2019-05-02T20:50:51Z

I am not an official maintainer.
If the code is legit but the official maintainer doesn't want it, I don't know what the best path forward would be. For me I forked it and I am using my fork. A bit of a hassle to stay up-to-date with changes as that becomes a very manual process, but open source software is hard!

k0nserv · 2019-05-03T09:28:14Z

Since there seems to be a signficant number of people who are interested in this feature we should probably merge it. @rethink-eder can you add this behaviour to the example app please?

…properties" test screen

ghost · 2019-05-03T17:31:21Z

@k0nserv I just committed a change in the screen "Setting text properties". We added a switch that will change the errorMessage position.

adrianod1as · 2019-05-13T17:02:00Z

+1

ghost · 2019-05-21T12:36:58Z

Hi @k0nserv did you have the opportunity to check the last commit with the screen example of this new property?

ghost · 2019-06-25T20:11:49Z

Hi @k0nserv . Would you approve this PR?

alexbtlv · 2019-06-26T03:57:57Z

+1

CaioSanchez · 2019-06-26T15:36:20Z

Any update on this?

olegdanu-newstore · 2019-08-06T06:19:54Z

Hey people :)
What is the status of this PR? What are the blockers to proceed on merging it?
What should be done to make this PR mergeable?

ghost · 2019-10-03T17:42:59Z

Hi @k0nserv. Are we ready for approve this PR?

ghost · 2019-12-18T14:01:40Z

Hi @k0nserv. Are we ready for approve this PR?

k0nserv · 2019-12-18T14:15:14Z

The build is failing atm so that needs to be fixed at the very least

ghost · 2019-12-18T22:16:42Z

Hi @k0nserv. The issue is now fixed. We merged from master and fixed the errors. Build is now working. Can you approve this PR?

k0nserv · 2019-12-30T13:55:20Z

.travis.yml

@@ -1,6 +1,8 @@
 language: objective-c
 xcode_project: SkyFloatingLabelTextField/SkyFloatingLabelTextField.xcodeproj
 xcode_scheme: SkyFloatingLabelTextField
+before_install:
+    - gem install bundler


Why is this needed? It should be handled by Travis already I believe

@k0nserv

This is because bundler is out to date in some of virtual machines. As here, for example: https://drive.google.com/file/d/10s3ZcLZz4kbYsNdCIdaQagPY1NqH9jcG/view?usp=sharing

Direct Link: https://travis-ci.org/Skyscanner/SkyFloatingLabelTextField/builds/626728997

The error message would be this one: https://drive.google.com/file/d/1IP429lcqvnJO-2vsPYJolQHbOeTVaar7/view?usp=sharing

Spencer Müller Diniz added 3 commits January 2, 2019 16:23

Make title and error labels independent.

cbfee17

Allow for error label to be displayed below the text field.

Code clean-up.

43a9aff

Added errorMessageAlignment option.

0778c01

ghost changed the title ~~Customizate errorMessage position~~ Customize errorMessage position Jan 29, 2019

jaredegan approved these changes Feb 15, 2019

View reviewed changes

Added comments for new public/open vars.

c228a9c

jaredegan reviewed Feb 18, 2019

View reviewed changes

Fixed errorLabel.textAlignment.

e008447

Added updateErrorLabelColor to updateColors.

348a556

Spencer Müller Diniz and others added 3 commits February 18, 2019 16:17

Fixed comment about errorColor property.

8f9f2ad

ErrorMessagePlacement: Changed some symbol names to be more Swifty, a…

807edec

…dded unit tests, adjust syntax to be consistent with project

Merge pull request #1 from gas-buddy/cusomizations-tweaks

f173076

Assorted adjustments to better fit SkyFloatingLabelTextField style

Add example of "errorMessagePlacement" property at the "Setting text …

f83f1bc

…properties" test screen

alkalouti1987 approved these changes Jun 30, 2019

View reviewed changes

junioreder added 3 commits December 18, 2019 11:18

Merge branch 'master' into customizations

981eafa

FIX swiftLint warnings and errors

365fa35

DEV Update bundler version

73ef34b

k0nserv reviewed Dec 30, 2019

View reviewed changes

k0nserv approved these changes Jan 7, 2020

View reviewed changes

k0nserv merged commit dd82b26 into Skyscanner:master Jan 7, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Customize errorMessage position #271

Customize errorMessage position #271

ghost commented Jan 29, 2019

jaredegan left a comment

jaredegan commented Feb 15, 2019

jaredegan commented Feb 15, 2019

jaredegan commented Feb 18, 2019

jaredegan Feb 18, 2019

spencerdiniz commented Feb 18, 2019

spencerdiniz commented Feb 18, 2019

jaredegan commented Feb 18, 2019

spencerdiniz commented Feb 18, 2019

k0nserv commented Feb 21, 2019

spencerdiniz commented Feb 21, 2019 •

edited

Loading

Bernix01 commented Apr 2, 2019

odanu commented Apr 7, 2019

ghost commented May 2, 2019 •

edited by ghost

Loading

jaredegan commented May 2, 2019

k0nserv commented May 3, 2019

ghost commented May 3, 2019

adrianod1as commented May 13, 2019

ghost commented May 21, 2019

ghost commented Jun 25, 2019

alexbtlv commented Jun 26, 2019

CaioSanchez commented Jun 26, 2019

olegdanu-newstore commented Aug 6, 2019

ghost commented Oct 3, 2019

ghost commented Dec 18, 2019

k0nserv commented Dec 18, 2019

ghost commented Dec 18, 2019

k0nserv Dec 30, 2019

ghost Jan 5, 2020 •

edited by ghost

Loading

Customize errorMessage position #271

Customize errorMessage position #271

Conversation

ghost commented Jan 29, 2019

jaredegan left a comment

Choose a reason for hiding this comment

jaredegan commented Feb 15, 2019

jaredegan commented Feb 15, 2019

jaredegan commented Feb 18, 2019

jaredegan Feb 18, 2019

Choose a reason for hiding this comment

spencerdiniz commented Feb 18, 2019

spencerdiniz commented Feb 18, 2019

jaredegan commented Feb 18, 2019

spencerdiniz commented Feb 18, 2019

k0nserv commented Feb 21, 2019

spencerdiniz commented Feb 21, 2019 • edited Loading

Bernix01 commented Apr 2, 2019

odanu commented Apr 7, 2019

ghost commented May 2, 2019 • edited by ghost Loading

jaredegan commented May 2, 2019

k0nserv commented May 3, 2019

ghost commented May 3, 2019

adrianod1as commented May 13, 2019

ghost commented May 21, 2019

ghost commented Jun 25, 2019

alexbtlv commented Jun 26, 2019

CaioSanchez commented Jun 26, 2019

olegdanu-newstore commented Aug 6, 2019

ghost commented Oct 3, 2019

ghost commented Dec 18, 2019

k0nserv commented Dec 18, 2019

ghost commented Dec 18, 2019

k0nserv Dec 30, 2019

Choose a reason for hiding this comment

ghost Jan 5, 2020 • edited by ghost Loading

Choose a reason for hiding this comment

spencerdiniz commented Feb 21, 2019 •

edited

Loading

ghost commented May 2, 2019 •

edited by ghost

Loading

ghost Jan 5, 2020 •

edited by ghost

Loading