Ticket #905 (closed planned_task: obsolete)

Opened 14 years ago

Last modified 13 years ago

TEXT_FONTS_INTERFACE_R0

Reported by: Astea Owned by: nenko
Priority: 3 Milestone: M05_PRE5
Component: TEXT_FORMATTING Version: 2.0
Keywords: Cc:
Category: MAIN Effort: 1
Importance: 0 Ticket_group:
Estimated Number of Hours: Add Hours to Ticket:
Billable?: Total Hours:
Analysis_owners: dido Design_owners: nenko
Imp._owners: nenko, nenko, nenko Test_owners:
Analysis_reviewers: nenko Changelog:
Design_reviewers: boyan Imp._reviewers: meddle, meddle, meddle, pap, meddle, pap, meddle
Test_reviewers: Analysis_score: 4
Design_score: 3.5 Imp._score: 3
Test_score: 0

Description

wiki page: TEXT_FONTS_INTERFACE_R0 - effort: 1d

Attachments

firstStyleException.txt (924.7 KB) - added by meddle 13 years ago.
The first exception that is thrown when you rapidly change styles.
secondStyleException.txt (1.6 MB) - added by meddle 13 years ago.
The second exception that is thrown when you rapidly change styles.
fontSizeException.txt (299.4 KB) - added by meddle 13 years ago.
navigationException.txt (421.1 KB) - added by meddle 13 years ago.

Change History

comment:1 Changed 14 years ago by dido

  • Owner changed from Astea to dido
  • Design_score set to 0
  • Imp._score set to 0
  • Test_score set to 0
  • Analysis_score set to 0

comment:2 Changed 14 years ago by dido

  • Status changed from new to s1a_analysis_started
  • Analysis_owners set to dido

comment:3 Changed 14 years ago by dido

  • Status changed from s1a_analysis_started to s1b_analysis_finished

comment:4 Changed 13 years ago by nenko

  • Status changed from s1b_analysis_finished to s1c_analysis_ok
  • Analysis_reviewers set to nenko
  • Analysis_score changed from 0 to 4

comment:5 Changed 13 years ago by nenko

  • Design_owners set to nenko
  • Owner changed from dido to nenko
  • Status changed from s1c_analysis_ok to s2a_design_started

comment:6 Changed 13 years ago by nenko

  • Status changed from s2a_design_started to s2b_design_finished

comment:7 Changed 13 years ago by boyan

  • Status changed from s2b_design_finished to s2c_design_ok
  • Design_score changed from 0 to 3.5
  • Design_reviewers set to boyan
  • The module name might not be appropriate. Probably a better suggestion is org.sophie2.main.func.text.gui - see the GROUP_PLUGIN_STRUCTURE_R0 task and discuss with pav the most appropriate name.
  • It is a good idea to link existing classes that are mentioned for easier reviewing.

Other than that the design seems ok. (30m)

comment:8 Changed 13 years ago by nenko

  • Status changed from s2c_design_ok to s3a_implementation_started

comment:9 Changed 13 years ago by nenko

  • Status changed from s3a_implementation_started to s3b_implementation_finished

comment:10 Changed 13 years ago by nenko

  • Imp._owners set to nenko

comment:11 Changed 13 years ago by meddle

  • Status changed from s3b_implementation_finished to s2c_design_ok
  • Imp._score changed from 0 to 2.5
  • Imp._reviewers set to meddle

The implementation works, but there are terrible problems with the memory. When you fix that the task will pass.

2.5p (20m)

comment:12 Changed 13 years ago by nenko

  • Status changed from s2c_design_ok to s3a_implementation_started

comment:13 Changed 13 years ago by nenko

  • Status changed from s3a_implementation_started to s3b_implementation_finished
  • Imp._owners changed from nenko to nenko, nenko

comment:14 Changed 13 years ago by meddle

  • Status changed from s3b_implementation_finished to s2c_design_ok
  • Imp._score changed from 2.5 to 2
  • Imp._reviewers changed from meddle to meddle, meddle

The implementation will fail because of the following reasons:

  • There are still problems when you change the styles rapidly, the frames freeze.
  • There was files without JavaDoc at all.
  • There were files for the chaining task with the files for this task, see changeset : 2958.
  • The code will be re-reviewed detailed when the above things are OK.

I advice you to create separate clean branches for all your text tasks.

2p (40m)

comment:15 Changed 13 years ago by nenko

  • Status changed from s2c_design_ok to s3a_implementation_started

comment:16 Changed 13 years ago by nenko

  • Status changed from s3a_implementation_started to s3b_implementation_finished
  • Imp._owners changed from nenko, nenko to nenko, nenko, nenko
  • new branch is added
  • the problems when you change styles rapidly are not solved
  • classes wuthout JavaDoc are removed
  • the changeset should cotaint only thigs related to this task

Changed 13 years ago by meddle

The first exception that is thrown when you rapidly change styles.

Changed 13 years ago by meddle

The second exception that is thrown when you rapidly change styles.

comment:17 Changed 13 years ago by meddle

  • Status changed from s3b_implementation_finished to s2c_design_ok
  • Imp._score changed from 2 to 1
  • Imp._reviewers changed from meddle, meddle to meddle, meddle, meddle, pap

The branch is in good state now. We took a deeper look at your implementation and saw a few more problems:

  • Problems with the code :
    • You have very strange conditions, for example : "if (preStyle.equals(HotStyleDef.EMPTY_STYLE) == false)"... Why didn't you write "if (!preStyle.equals(HotStyleDef.EMPTY_STYLE))"?
    • Fix the name of your test, now it's called "TextStlyerTest".
    • Use assertEquals instead of "assertTrue(something.equals(otherthing));".
    • Use assertEquals instead of "assertTrue(somePrimitivevalue == otherPrimitiveValue);".
    • Test NaiveStyler#replaceStyle even if you don't use it now.
    • The sort key of TextFontHaloButton's VisualElementDef ("text-font-halo-button") is not a good sort key, usualy these sort keys start with three lower letters_dash_something. Example "aaa-text-font-halo-button".
    • The same as above applyes for TextFontHaloMenu, TextFontFamily, TextFontSize, TextItalicStyle, TextBoldStyle, TextUnderlineStyle, TextStrikethroughStyle and TextFontHud.
  • Problems with the behavior:
    • The controls in the hud should be initialized depending on the underlying model text and its style. This includes the selected font/font_size in the combo boxes.
    • The later should change when the hud is attached to a new frame.
  • Problems with exceptions:
    • The exceptions when you rapidly change styles are attached to the ticked this time.
      • firstStyleException.txt
      • secondStyleException.txt

1p (80m)

comment:18 Changed 13 years ago by nenko

  • Status changed from s2c_design_ok to s3a_implementation_started

comment:19 Changed 13 years ago by nenko

  • Status changed from s3a_implementation_started to s3b_implementation_finished

well, another try...

Changed 13 years ago by meddle

Changed 13 years ago by meddle

comment:20 Changed 13 years ago by meddle

  • Status changed from s3b_implementation_finished to s2c_design_ok
  • Imp._score changed from 1 to 2
  • Imp._reviewers changed from meddle, meddle, meddle, pap to meddle, meddle, meddle, pap, meddle, pap
  • Problems With the behavior and exceptions:
    • When trying to change the font size, exception is thrown; See the attachment "fontSizeException.txt". I wrote "anime" and tryed to change it's size. (That is if the cursor is not visible...)
    • The same as above applies to the use-cases when you try to change bold, italic, etc.. all of the styles, so practically the styles don't work.
    • When there is no text in the Frame you can change the styles, but the font size returns to "12" every time when you click in some of the checkboxes for the other styles or change the font name. The same applyes and for the font name itself.
    • The row of the hud that contains the "font name: " and the BoundComboBox is to long and its ends are out of the hud. (That's only under windows...)
    • Now when you navigate through the text with the keyboard arrows, an exception is thrown. See the attachment "navigationException.txt".
  • Code notes:
    • You fixed the strange condition checks in you test, and the test itself is super. In the code you left them like in the NaiveHotText. But that thing is not mandatory...

This implementation like code is good, but has many bug, the bug with the memory is not present anymore, and there is some strange one with the layout (the one from the previous review) but it is triggered hard and rarely, so if you fix the new bugs from the above section the implementation will pass.

2p (70м)

comment:21 Changed 13 years ago by nenko

  • Status changed from s2c_design_ok to s3a_implementation_started

comment:22 Changed 13 years ago by nenko

  • Status changed from s3a_implementation_started to s3b_implementation_finished

comment:23 Changed 13 years ago by meddle

  • Status changed from s3b_implementation_finished to s3c_implementation_ok
  • Imp._score changed from 2 to 3
  • Imp._reviewers changed from meddle, meddle, meddle, pap, meddle, pap to meddle, meddle, meddle, pap, meddle, pap, meddle
  • There is the assertation error when changing to some of the fonts and they are with size over 13.
  • You left a system out :)

3p (2h)

comment:24 Changed 13 years ago by deyan

  • Status changed from s3c_implementation_ok to closed
  • Resolution set to obsolete

Batch update from file query-obsoleted.csv

Note: See TracTickets for help on using tickets.