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

TIMOB-8821: BlackBerry: Implement UI.Label #61

Merged

Conversation

alexandergalstyan
Copy link

Reviewers: JP, HarutM

ChangeLog:

  • Added new NativeAbstractTextControlObject as a parent for Label, TextField, TextArea controls.
  • Implemented font, textAlign, color properties, methods
  • Move appropriate methods from Label control into the parent
  • Updated yaml doc for Label

Test Cases:

  • Build tibb.
  • Build tibbtest.
  • In the java script, instance label object with font, textAlign, color properties.
  • Make sure existing mentioned properties works.
  • Run docgen.py script
  • Make sure documentation includes appropriates changes for blackberry platform

Reviewers: JP, HarutM

ChangeLog:
- Added new NativeAbstractTextControlObject as a parent for Label, TextField, TextArea controls.
- Implemented font, textAlign, color properties, methods
- Move appropriate methods from Label control into the parent
- Updated yaml doc for Label

Test Cases:
- Build tibb.
- Build tibbtest.
- In the java script, instance label object with font, textAlign, color properties.
- Make sure existing mentioned properties works.

- Run docgen.py script
- Make sure documentation includes appropriates changes for blackberry platform
#include <bb/cascades/controls/abstracttextcontrol.h>
#include <bb/cascades/controls/textalignment.h>
#include <qt4/QtCore/qmap.h>
#include <qt4/QtCore/qstring.h>
Copy link

Choose a reason for hiding this comment

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

You only need to include the class name for Qt classes, like so:

#include <QMap>
#include <QString>

I don't think there's an equivalent for cascades classes though

Copy link
Author

Choose a reason for hiding this comment

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

Done

@Harutyun
Copy link

Reviewed


private:
bb::cascades::AbstractTextControl* textControl_;
};
Copy link

Choose a reason for hiding this comment

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

Also disable the copy ctor and assignment operator

Copy link
Author

Choose a reason for hiding this comment

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

Done

@jpl-mac
Copy link

jpl-mac commented May 25, 2012

Reviewed

unknown added 2 commits May 29, 2012 12:36
Reviewers: JP, HarutM

ChangeLog:
- Minor fixes
- Update NativeTextFieldObject to be derived from NativeAbstractTextControlObject
- Move string constants to defines
- Add width, height properties to native control object

Test Cases:
-Build tibb.
- Build tibbtest.
- In the java script, instance label object with font, textAlign, color properties.
- Make sure existing mentioned properties works.

- Run docgen.py script
- Make sure documentation includes appropriates changes for blackberry platform
@alexandergalstyan
Copy link
Author

Updated


{
"height", TI_PROP_PERMISSION_READ | TI_PROP_PERMISSION_WRITE,
N_PROP_HEIGHT
}
Copy link

Choose a reason for hiding this comment

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

These should be in order, can you please order the ones that were added to the end?

Copy link
Author

Choose a reason for hiding this comment

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

Updated.

@jpl-mac
Copy link

jpl-mac commented May 29, 2012

Reviewed

Reviewers: JP, HarutM

ChangeLog:
- Address comments

Test Cases:
- Build tibb.
- Build tibbtest.
- In the java script, instance label object with font, textAlign, color properties.
- Make sure existing mentioned properties works.

- Run docgen.py script
- Make sure documentation includes appropriates changes for blackberry platform
@alexandergalstyan
Copy link
Author

updated

textControl_->textStyle()->setAlignment(bb::cascades::TextAlignment::ForceRight);
break;
default:
qDebug() << "Unknown value recieved: " << value;
Copy link

Choose a reason for hiding this comment

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

s/recieved/received/

Also log the class name and method name, so the origin of the log can be found easily. Goes for all logging messages

@jpl-mac jpl-mac closed this Jun 1, 2012
@jpl-mac jpl-mac reopened this Jun 1, 2012
@jpl-mac
Copy link

jpl-mac commented Jun 1, 2012

For #61 (comment) , the constants defined as javascript constants.
Please show me where that is done as I must have missed it, I don't see where that is done.

For #61 (comment) , still waiting for Suavek response, since cannot find appropriate docs for that

The Font doc still needs to be updated to reflect the work of this PR. Not all of it needs answers from Suavek. You can add notes to Jira for things that will need to be updated once we get answers from Suavek. You can still update the Font.yml file for this PR with the information that you have, unless you prefer to delay this patch until you get the answers.

  • fontFamily: Do you know which fonts are supported currently? (Question sent to Suavek) Does that mean that setting fontFamily currently doesn't work? If so this must be captured in Jira to be updated at a later time when we get answers

The rest of the properties you can update now

  • fontSize: what unit does the BB use? should the unit be appended to the number?
  • fontStyle: does italic work? currently it says only supported for iphone ipad
  • fontWeight: do bold and semibold work?
    If you've tested your work you should already know most of these answers

If there are still things that need to be updated but need to wait for R6 to find the answers then add them to the JIra Issue for reworking label/font after R6

@jpl-mac
Copy link

jpl-mac commented Jun 1, 2012

What is ver5? I don't think you understand defining read-only constants in the javascript API.

The user needs to be able to do something like

var v = Ti.UI.TEXT_ALIGNMENT_CENTER;

I don't see that from the code.

@jpl-mac
Copy link

jpl-mac commented Jun 1, 2012

Concerning the read-only vars, you can talk to DavidC about how to go about adding the constants in V8. Once that's done, you have to update the Ti.UI yml file. It contains the following line for the TEXT_ALIGNMENT_* properties:

This constant is a string on android, and a number on iOS and mobileweb.

It needs to be updated to specify the type for the constant on BB. I expect that it will be a number.

Reviewers: JP, HarutM

ChangeLog:
- Updated Font.yml
- Updated UI.yml
- Added text alignment javascript constants

Test Cases:
- Build tibb.
- Build tibbtest.
- In the java script, instance label object with font, textAlign, color properties.
- Make sure existing mentioned properties works.

- Run docgen.py script
- Make sure documentation includes appropriates changes for blackberry platform
@alexandergalstyan
Copy link
Author

Updated

@@ -61,6 +61,9 @@ description: |

* [Custom Fonts in the Titanium Mobile Guides](https://wiki.appcelerator.org/display/guides/Custom+Fonts)

* On BlackBerry, fontSize use post script units. Valid ranges are >= 0. Sizes < 0 will be clamped to 0.
fontWeight valid values are "bold" or "normal."
Copy link

Choose a reason for hiding this comment

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

Thanks Alex,

This should be in the fontSize specific description. As for the fontWeight, it matches what's already in the description.

Copy link
Author

Choose a reason for hiding this comment

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

Done

Reviewers: JP, HarutM

ChangeLog:
- Updated yaml doc for Font, UI
- Minor fixes

Test Cases:
- Build tibb.
- Build tibbtest.
- In the java script, instance label object with font, textAlign, color properties.
- Make sure existing mentioned properties works.

- Run docgen.py script
- Make sure documentation includes appropriates changes for blackberry platform
@alexandergalstyan
Copy link
Author

Updated

@@ -92,6 +91,8 @@ properties:
For example, "16dp" specifies a size of 16 density-independent pixels.

iOS ingores any unit specifier after the size value.

On BlackBerry, fontSize use post script units. Valid ranges are >= 0. Sizes < 0 will be clamped to 0.
Copy link

Choose a reason for hiding this comment

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

Should also specify that we value should be a Number. String currently also work but only if not followed by unit specifier (either we have it ignore the unit specifiers like for iOS or update the doc to specify that it only takes a number).

Copy link
Author

Choose a reason for hiding this comment

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

Done

Reviewers: JP, HarutM

ChangeLog:
- Update Font.yml

Test Cases:
- Build tibb.
- Build tibbtest.
- In the java script, instance label object with font, textAlign, color properties.
- Make sure existing mentioned properties works.

- Run docgen.py script
- Make sure documentation includes appropriates changes for blackberry platform
TEXT_ALIGNMENT_CENTER,
TEXT_ALIGNMENT_RIGHT
};

Copy link

Choose a reason for hiding this comment

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

Moving this causes a compile error because NativeAbstractTextControlObject.cpp still references these.

After discussing this with David C, we came to the conclusion that we should have a new file named TiConstants.h to contain all the constants definitions that will be needed throughout the TI api. This file will then be included in both places. The Constants should also be declared in a Ti namespace to avoid name clashes.

There is a macro that we'll use to not have to write the .cpp manually.

unknown added 2 commits June 6, 2012 13:10
…ry' into labelBranch

Conflicts:
	blackberry/tibb/NativeControlObject.cpp
	blackberry/tibb/NativeControlObject.h
	blackberry/tibb/NativeTextFieldObject.cpp
	blackberry/tibb/NativeTextFieldObject.h
	blackberry/tibb/TiUIBase.cpp
Reviewers: JP, HarutM

ChangeLog:
- Added TiConstants.h file to containt all Ti constants.

Test Cases:
- Build tibb.
- Build tibbtest.
- In the java script, instance label object with font, textAlign, color properties.
- Make sure existing mentioned properties works.

- Run docgen.py script
- Make sure documentation includes appropriates changes for blackberry platform
@alexandergalstyan
Copy link
Author

Merged with latest and updated.


On BlackBerry, fontSize use post script units. Valid ranges are >= 0. Sizes < 0 will be clamped to 0.
fontSize value should be a Number. String currently also work but only if not followed by unit specifier.
BlackBerry ingores any unit specifier after the size value.
Copy link

Choose a reason for hiding this comment

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

Change fontSize value should be a Number. to fontSize value is a Number.
Remove String currently also work but only if not followed by unit specifier. that was just a note i was telling you
Remove BlackBerry ingores any unit specifier after the size value. as it is not true, currently if the fontSize is a string with a unit specifier the conversion fails, it does not ignore the unit specifier

@alexandergalstyan
Copy link
Author

Updated

Reviewers: JP, HarutM

ChangeLog:
- Update apidoc for Font and TextField.

Test Cases:
- Build tibb.
- Build tibbtest.
- In the java script, instance label object with font, textAlign, color properties.
- Make sure existing mentioned properties works.

- Run docgen.py script
- Make sure documentation includes appropriates changes for blackberry platform
@jpl-mac
Copy link

jpl-mac commented Jun 6, 2012

Approved

TIMOB-8821: BlackBerry: Implement UI.Label
alexandergalstyan added a commit that referenced this pull request Jun 6, 2012
TIMOB-8821: BlackBerry: Implement UI.Label
@alexandergalstyan alexandergalstyan merged commit 5156a9d into Macadamian:blackberry Jun 6, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants