[TIMOB-24080] : iOS - Paragraph style support in attributed string #10032

vijaysingh-axway · 2018-05-09T06:14:36Z

https://jira.appcelerator.org/browse/TIMOB-24080

hansemannn

Some minor issues.

hansemannn · 2018-05-09T10:30:29Z

iphone/Classes/UIModule.m

@@ -594,6 +594,26 @@ - (NSNumber *)ATTRIBUTE_LINE_BREAK_BY_TRUNCATING_MIDDLE
 {
  return NUMINTEGER(NSLineBreakByTruncatingMiddle);
 }
+- (NSNumber *)ATTRIBUTE_TEXT_ALIGNMENT_LEFT
+{
+  return NUMINTEGER(NSTextAlignmentLeft);


We already support these on the UIModule class via Ti.UI.TEXT_ALIGNMENT_*

hansemannn · 2018-05-09T10:30:54Z

iphone/Classes/UIModule.m

@@ -504,7 +504,7 @@ - (NSNumber *)convertUnits:(id)args
 MAKE_SYSTEM_PROP(ATTRIBUTE_STRIKETHROUGH_COLOR, AttributeNameStrikethroughColor);
 MAKE_SYSTEM_PROP(ATTRIBUTE_OBLIQUENESS, AttributeNameObliqueness);
 MAKE_SYSTEM_PROP(ATTRIBUTE_EXPANSION, AttributeNameExpansion);
-MAKE_SYSTEM_PROP(ATTRIBUTE_LINE_BREAK, AttributeNameLineBreak);
+MAKE_SYSTEM_PROP(ATTRIBUTE_LINE_BREAK, AttributeNameLineBreak); // deprecated


It looks like we only warn the user via code so far, not via docs. Please add.

hansemannn · 2018-05-09T10:32:11Z

apidoc/Titanium/UI/Attribute.yml

+    description: |
+      This property contains the line break mode to be used laying out the text of paragraph.
+    type: Number
+    constants: [ Titanium.UI.ATTRIBUTE_LINE_BREAK_* ]


I thought this is deprecated?

jquick-axway · 2018-05-09T19:34:44Z

apidoc/Titanium/UI/Attribute.yml

+      If positive, this value is the distance from the leading margin 
+      (for example, the left margin in left-to-right text). If 0 or negative, 
+      it is the distance from the trailing margin. This is a float value.
+    type: Number


We need to document what units tailIndex uses, such as points.

jquick-axway · 2018-05-09T19:35:22Z

apidoc/Titanium/UI/Attribute.yml

+    description: |
+      This value is always nonnegative. This value is included in the line fragment heights 
+      in the layout manager.
+    type: Number


We need to document the lineSpacing units, such as points.

jquick-axway · 2018-05-09T19:37:58Z

apidoc/Titanium/UI/Attribute.yml

+      would not otherwise fit in the available space. The maximum amount of tightening performed 
+      by the system is dependent on the font, line width, and other factors. The default value of 
+      this property is NO.
+    type: Number


The allowsDefaultTighteningForTruncation type should be boolean, not number.

And the description should use true and false instead of the Objective-C YES and NO.

jquick-axway · 2018-05-09T19:41:27Z

apidoc/Titanium/UI/Attribute.yml

+      to separate it from the following paragraph. This value must be nonnegative. The space 
+      between paragraphs is determined by adding the paragraphSpacing of previous paragraph and 
+      the paragraphSpacingBefore of current paragraph.
+    type: Number


Perhaps we should rename property paragraphSpacing to paragraphSpacingAfter. This makes it clear that it's only applied after the paragraph (assuming this is actually how it works). And it clearly separates it from the paragraphSpacingBefore property.

jquick-axway · 2018-05-09T19:52:11Z

@vijaysingh-axway, @hansemannn, it's kind of a bummer that we're hard-coding properties such as lineSpacing to points/dips instead of respecting the default units set for the app. This can make view layout/alignment with text a pain.

To be fair, I've noticed that our existing ATTRIBUTE_BASELINE_OFFSET is hard-coded to pixels and ATTRIBUTE_KERN is hard-coded to points. So, it appears to be a pre-existing issue with attributed strings, but do we really want to continue this trend? Perhaps allowing these properties to be set to a string where the dev can set the units might be better? (If you feel that it's too late, then fair enough.)

jquick-axway · 2018-05-09T20:01:28Z

apidoc/Titanium/UI/Attribute.yml

+      The natural line height of the receiver is multiplied by this factor (if positive) 
+      before being constrained by minimum and maximum line height. The default value of 
+      this property is 0.0.
+    type: Number


The docs here don't do a good job describing what lineHeightMultiple, maximumLineHeight, and minimumLineHeight do. Nor why you would want to use them.
(Not trying to be rude. Just looking at this from a Titanium dev's perspective.) 😄

I assume you would want to use minimumLineHeight in case you use mixed font sizes and you want all lines to have the same height as the largest font size. That's the only use-case I can think of.

But why would you want to use maximumLineHeight? Would it clip text that exceed this height? If there is answer to this, then we should document it.

Also, I have no clue what lineHeightMultiple is supposed to do from reading the description... nor why I would want to use it. If there is a valid use-case, then let's document it.

Also, I assume a value of zero for maximumLineHeight means do not apply a max, right? Otherwise text would always be clipped. We should document that.

I assume the same is true for lineHeightMultiple? A value of zero means don't multiply?

hansemannn · 2018-05-09T20:41:11Z

I agree! There is a helper for convertig values to dimensions. Vijay, can you address that?

hansemannn · 2018-05-15T09:31:57Z

@vijaysingh-axway Can you check the review comments? Moving to 7.3.0 for now, due to lower priority.

jquick-axway · 2018-05-15T18:25:36Z

Note that I'm not trying to picky. These details matter when we implement this on Android for parity. Documenting these details not only benefits Titanium devs, but it also serves as a specification when implementing this feature on other platforms. Thanks!

hansemannn · 2018-08-20T09:21:48Z

iphone/Classes/TiUIAttributedStringProxy.m

@@ -178,11 +213,12 @@ - (void)addAttribute:(id)args
    break;

  case AttributeNameLineBreak:
+    DEPRECATED_REPLACED(@"UI.ATTRIBUTE_LINE_BREAK", @"UI.ATTRIBUTE_PARAGRAPH_STYLE.lineBreakMode", @"7.3.0");


vijaysingh-axway · 2018-08-21T10:39:27Z

@hansemannn Fixed the dimension conversion. Now you can review. Thanks!

hansemannn · 2018-08-21T10:40:53Z

@vijaysingh-axway You still should update the docs to reflect that developers can also use non-Number types.

build · 2018-08-22T06:37:47Z

	Messages
📖	💾 Here's the generated SDK zipfile.

Generated by 🚫 dangerJS

hansemannn

CR passed!

hansemannn · 2018-08-22T06:47:52Z

@jquick-axway Can you re-review / approve this?

jquick-axway

CR: Pass

keerthi1032 · 2018-09-19T18:56:12Z

FR passed. Attributed string Works fine on IOS. PR merged
Test Environment:
Name = Mac OS X
Version = 10.13.6
Architecture = 64bit
Memory = 17179869184
Node.js
Node.js Version = 8.9.1
npm Version = 5.5.1
Titanium CLI
CLI Version = 5.1.1
Titanium SDK
SDK Version = 7.5.0 local sdk
iPhone 6s -ios 12
iPhone 5 -ios 11 simulator

vijaysingh-axway added 3 commits May 8, 2018 15:13

[TIMOB-24080] : iOS - Paragraph style support in attributed string

6b381e5

[TIMOB-24080] : Doc added

1f3cd18

[TIMOB-24080] : iOS paragraph style support to AttributedString

9b2613d

vijaysingh-axway requested a review from hansemannn May 9, 2018 06:14

Merge branch 'master' into TIMOB-24080

0f35540

build added docs ios needs tests 🚨 labels May 9, 2018

[TIMOB-24080] : Indentation fixed for doc

e812a87

build added docs ios needs tests 🚨 labels May 9, 2018

hansemannn requested changes May 9, 2018

View reviewed changes

jquick-axway suggested changes May 9, 2018

View reviewed changes

jquick-axway reviewed May 9, 2018

View reviewed changes

hansemannn mentioned this pull request May 15, 2018

[TIMOB-24080] Implementing ParagraphStyle Ti.UI.AttributedString #8558

Closed

hansemannn added this to the 7.3.0 milestone May 15, 2018

sgtcoolguy modified the milestones: 7.3.0, 7.4.0 May 16, 2018

vijaysingh-axway added 4 commits May 23, 2018 15:11

Merge branch 'master' into TIMOB-24080

eff5079

Merge branch 'master' into TIMOB-24080

18cfc04

[TIMOB-24080] : Updated docs

126efc4

[TIMOB-24080] : Unite test added

b39ec57

[TIMOB-24080] : Unit test linting issue fixed

7e6498e

tidev deleted a comment from build Aug 15, 2018

vijaysingh-axway and others added 3 commits August 15, 2018 17:27

[TIMOB-24080] : Final linting fixed

67ca65f

Update ti.ui.attributedstring.addontest.js

103c545

Merge branch 'master' into TIMOB-24080

b912dac

hansemannn reviewed Aug 20, 2018

View reviewed changes

hansemannn removed the hold for parity ✋ This PR should not be merged until an equivalent PR is ready for the other platform(s) label Aug 21, 2018

Update TiUIAttributedStringProxy.m

f6a06e8

hansemannn modified the milestones: 7.4.0, 7.5.0 Aug 21, 2018

vijaysingh-axway added 2 commits August 21, 2018 18:27

[TIMOB-24080] : iOS - Paragraph style support in attributed string

2e13750

Merge branch 'master' into TIMOB-24080

760b9fb

[TIMOB-24080] : Doc updated

2396835

hansemannn added 4 commits August 22, 2018 08:46

Update Attribute.yml

f084a52

Update UI.yml

d1e3234

Update TiUIAttributedStringProxy.m

fc95b6f

Merge branch 'master' into TIMOB-24080

4ce0309

hansemannn approved these changes Aug 22, 2018

View reviewed changes

Merge branch 'master' into TIMOB-24080

a72382e

jquick-axway approved these changes Sep 1, 2018

View reviewed changes

vijaysingh-axway and others added 3 commits September 3, 2018 09:52

Merge branch 'master' into TIMOB-24080

bd42f0b

Merge branch 'master' into TIMOB-24080

c733c1f

Merge branch 'master' into TIMOB-24080

a69174a

keerthi1032 merged commit 7561912 into tidev:master Sep 19, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[TIMOB-24080] : iOS - Paragraph style support in attributed string #10032

[TIMOB-24080] : iOS - Paragraph style support in attributed string #10032

vijaysingh-axway commented May 9, 2018 •

edited

Loading

hansemannn left a comment

hansemannn May 9, 2018

hansemannn May 9, 2018

hansemannn May 9, 2018

jquick-axway May 9, 2018

jquick-axway May 9, 2018

jquick-axway May 9, 2018

jquick-axway May 9, 2018

jquick-axway commented May 9, 2018

jquick-axway May 9, 2018 •

edited

Loading

jquick-axway May 9, 2018

hansemannn commented May 9, 2018

hansemannn commented May 15, 2018

jquick-axway commented May 15, 2018

hansemannn Aug 20, 2018

vijaysingh-axway commented Aug 21, 2018

hansemannn commented Aug 21, 2018

build commented Aug 22, 2018 •

edited

Loading

hansemannn left a comment

hansemannn commented Aug 22, 2018

jquick-axway left a comment

keerthi1032 commented Sep 19, 2018 •

edited

Loading

[TIMOB-24080] : iOS - Paragraph style support in attributed string #10032

[TIMOB-24080] : iOS - Paragraph style support in attributed string #10032

Conversation

vijaysingh-axway commented May 9, 2018 • edited Loading

hansemannn left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jquick-axway commented May 9, 2018

jquick-axway May 9, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hansemannn commented May 9, 2018

hansemannn commented May 15, 2018

jquick-axway commented May 15, 2018

Choose a reason for hiding this comment

vijaysingh-axway commented Aug 21, 2018

hansemannn commented Aug 21, 2018

build commented Aug 22, 2018 • edited Loading

hansemannn left a comment

Choose a reason for hiding this comment

hansemannn commented Aug 22, 2018

jquick-axway left a comment

Choose a reason for hiding this comment

keerthi1032 commented Sep 19, 2018 • edited Loading

vijaysingh-axway commented May 9, 2018 •

edited

Loading

jquick-axway May 9, 2018 •

edited

Loading

build commented Aug 22, 2018 •

edited

Loading

keerthi1032 commented Sep 19, 2018 •

edited

Loading