Ticket #2251 (closed tweak: obsolete)

Opened 10 years ago

Last modified 8 years ago

caret-not-following-chain -- Text caret does not go to the prev/next chained frame

Reported by: deyan Owned by: diana
Priority: major Milestone: X3
Component: uncategorized Version: 2.0
Keywords: Cc:
Category: unknown Effort:
Importance: 88 Ticket_group: text
Estimated Number of Hours: 0 Add Hours to Ticket: 0
Billable?: yes Total Hours: 0
Analysis_owners: deyan Design_owners: diana
Imp._owners: diana Test_owners:
Analysis_reviewers: diana Changelog: Changelog
Design_reviewers: meddle Imp._reviewers: deyan, pap
Test_reviewers: Analysis_score: 3
Design_score: 3 Imp._score: 3.5
Test_score: 0

Description (last modified by kyli) (diff)

  • Make the text caret go to next/previous frame (and page if needed).
  • The frame containing the caret should be focused.
  • If you select from right to left, the caret should be at the beginning of the selection.


2251.patch (195.4 KB) - added by diana 10 years ago.

Change History

comment:1 Changed 10 years ago by deyan

  • Description modified (diff)

comment:2 Changed 10 years ago by deyan

  • Owner set to deyan
  • Status changed from new to s1a_analysis_started
  • Description modified (diff)

comment:3 Changed 10 years ago by deyan

  • Status changed from s1a_analysis_started to s1b_analysis_finished

comment:4 Changed 10 years ago by diana

  • Design_owners set to diana
  • Imp._owners set to diana
  • Analysis_reviewers set to diana
  • Analysis_score changed from 0 to 5

comment:5 Changed 10 years ago by diana

  • Status changed from s1b_analysis_finished to s1c_analysis_ok

comment:6 Changed 10 years ago by deyan

  • Importance set to 88

comment:7 Changed 10 years ago by deyan

  • Milestone set to X3

comment:8 Changed 10 years ago by deyan

  • Description modified (diff)

Batch update from file active_tickets.csv

comment:9 Changed 10 years ago by kyli

  • Ticket_group set to text
  • Description modified (diff)
  • Changelog set to [wiki:Changelog]

comment:10 Changed 10 years ago by diana

  • Owner changed from deyan to diana
  • Status changed from s1c_analysis_ok to s2a_design_started

In order to provide the wanted functionality the following changes were made:

  • In org.sophie2.base.model.text.elements package make a new class CaretInfo - a class representing the information about the caret. Has a caret index and area index field. Used when navigating in the text. This class has a default constructor - public CaretInfo(int areaIndex, int caretIndex) and getters for the area index (public int getAreaIndex()) and for the caret index (public int getCaretIndex()).
  • In TextModelLogic create an inner enum: public enum EventIds - The operations for the TextModelLogic. It has one field: SELECT_VIEW with @EventParams({Integer.class}).

The event parameter is the area index in case of chaining which sholud be selected after navigating, deleting ot inserting text.

  • In TextModelLogic class make the getPos function return Object of type CaretInfo. The area index of this object is used to fire another event to select the proper view.
  • In TextModelLogic class make setSelection function fire another event to select the wanted view.
  • In HotTextLogic class add ON_SELECT_VIEW - handles the cases when the view is changed (when navigating/editing in chains).

comment:11 Changed 10 years ago by diana

  • Status changed from s2a_design_started to s2b_design_finished

comment:12 Changed 10 years ago by meddle

  • Status changed from s2b_design_finished to s2c_design_ok
  • Design_score changed from 0 to 3
  • Design_reviewers set to meddle
  • Analysis_score changed from 5 to 3
  • There is nothing about this isAutoChainMode parameter when creating the text action.
  • Why are you changing the fuzz factor
  • The design is not full for me but I am passing it to not waste your time, I expect detailed design notes for everything changed in the implementation section.
  • Name unit test appropriately - {name}Test
    • The annotations @before and @after @test are important.
    • When you change something in a base test class, I must give explanation why.
  • You CAN NOT score analysis without review comments especially with the maximum score.


comment:13 Changed 10 years ago by diana

  • Status changed from s2c_design_ok to s3a_implementation_started
  • In HeadTextFrameH class : createTextFrameAction function add another parameter - isAutoChainMode (true if the created frame will be in AUTO_CHAIN mode).
  • In HotLayout class change the LAZY_FACTOR to 1 in order to force the layout to layout the text one frame before and after the current one (needed when deleting/ inserting in the beginning/ end of a chained frame).
  • In TextUtils : findPrevChar check if the given pos is the end pos and advance backwards if so(needed when the TextFontHaloMenu gets its styles from the text).
  • In TextUtils : findNextChar if the given index is the end of the text then the start index should be returned.
In TextFrameLogic class every assert ImmTextUtils.isIndexInText(pos, text) line is not correct because thete is another case which handles the end position of the text (checks if the returned unit == null), so every line of this type is replaced with : assert ImmTextUtils.isIndexInText(pos, text)
pos == text.getEnd();
  • In TextModelLogic : setSelection function remove the ImmTextInterval parameter andd add another two - one for the mark and one for the caret position (this is needed because the constructor of the ImmTextInterval reverts the first and second point if they are in reverse order but the mark and caret should not be reversed).

And as for the analysis it's the BEST I'VE EVER READ !!!! :) and I'm speechless....

Changed 10 years ago by diana

comment:14 Changed 10 years ago by diana

  • Status changed from s3a_implementation_started to s3b_implementation_finished

comment:15 Changed 10 years ago by pap

  • Status changed from s3b_implementation_finished to s3c_implementation_ok
  • Imp._score changed from 0 to 3.5
  • Imp._reviewers set to deyan, pap
  • Commited in [9156].
  • Improper JavaDoc of the test :) And you stated boyan as the author...
  • Don't swallow exceptions. This applies to tests, too. Actually in tests it is best to just put a throws statement.
  • Isn't "textModel.getCaret() - textModel.getMark() <= 0" the same as "textModel.getCaret() <= textModel.getMark()" (TextModelLogic)?
  • Some very nice paragraph related changes in TextModelLogic, that weren't mentioned anywhere. :)
  • Hah, you just swapped two segments of code.
  • Oh and I really enjoy the DOC_HOME and DOC_END navigation.

comment:16 Changed 8 years ago by meddle

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

Closing all the tickets before M Y1

Note: See TracTickets for help on using tickets.