Ticket #2323 (closed planned_task: obsolete)

Opened 10 years ago

Last modified 8 years ago

redesign character hud in order to improve space utilization in Sophie.

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

Description (last modified by dido) (diff)

redesign paragraph hud in order to improve space utilization in Sophie. See the attachment. This task is part of #2222

  • Change Style check boxes to skinable toggle buttons and align them beneath the font Combo box
  • The color buttons should be skinned as in the attachment and align with the left part of the size combo box
  • Reduce hud dimentions

Attachments

textCharacterHud.png (4.1 KB) - added by dido 10 years ago.

Change History

comment:1 Changed 10 years ago by deyan

  • Component changed from uncategorized to ui_consistency
  • Milestone set to X3

Batch update from file query.csv

Changed 10 years ago by dido

comment:2 Changed 10 years ago by dido

  • Keywords Sophie 2.x added
  • Type changed from bug to planned_task
  • Description modified (diff)
  • Changelog set to [wiki:Changelog]

comment:3 Changed 10 years ago by dido

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

comment:4 Changed 10 years ago by dido

  • Status changed from s1a_analysis_started to s1b_analysis_finished

comment:5 Changed 10 years ago by dido

  • Summary changed from redesign paragraph hud in order to improve space utilization in Sophie. to redesign character hud in order to improve space utilization in Sophie.

comment:6 Changed 10 years ago by deyan

  • Status changed from s1b_analysis_finished to s1c_analysis_ok
  • Analysis_reviewers set to deyan
  • Analysis_owners set to dido
  • Analysis_score changed from 0 to 3.5

The picture explains better what will be done

comment:7 Changed 10 years ago by dido

  • Status changed from s1c_analysis_ok to s2a_design_started

comment:8 follow-up: ↓ 9 Changed 10 years ago by dido

all changes are include inside svn://sophie2.org/sophie2/branches/private/dido/2323CharHudRed/[[BR]]
Note that this code resolves 2322 as well. There are still some problems with spacing in order to make it more consistent. Also some more comments might be useful.

comment:9 in reply to: ↑ 8 Changed 10 years ago by dido

  • Status changed from s2a_design_started to s2b_design_finished

comment:10 Changed 10 years ago by pap

  • Status changed from s2b_design_finished to s2c_design_ok
  • Design_score changed from 0 to 3
  • Design_reviewers set to pap
  • Generally the idea is nice
  • There is a problem with the space under the bottom control of the paragraph hud. I and deyan think that there should be a little more space below it.
  • The sentence "There are still some problems with spacing in order to make it more consistent" doesn't give much information.
  • Actually what you describe in the design gives no idea about the design itself. So I had to read through the code to get the idea.
  • And I like the new appearance of the HUDs.
  • Well if you have done both tickets, you should've changed the statuses of both of them.
  • There is commented code in HudDialog in the swingComponent property. Please make sure that the code is clean of unneeded commented out code.
  • Write the JavaDoc properly - first the comment, then an empty line and then the author TextParagraphHud.TextParagraphPropertiesContainer, TextFontHud.FontPropertiesContainer.
  • Add yourself as an author(with a new @author on a new line) to the TextParagraphHud, as you changed it alot.
  • The setup method of the TextParagraphHud.TextParagraphPropertiesContainer,.swingComponent() property is useless this way as it doesn't depend ot pro-lib. Its contents better fit the create method. I suppose that the revalidate and repaint calls are not needed when you change the layout manager. Also it is not nice to set the visibility together with adding the controls as this would make lots of unnecessary computation.
  • Don't put comma in the end of the list in the @RelatedChildren annotation as this will not compile with sun java compiler and only with the eclipse one.
  • I think that it is better to put some title in the skin for TextParagraphHud.TextParagraphPropertiesContainer.TextParagraphLineSpacing instead of just an empty string. Otherwise just don't put anything.
  • Dont "return new BoundValidation(Status.OK, "OK");", but use "return BoundValidation.SUCCESS;" instead - this is in the same class as the previous bullet. And for all other combo boxes.
  • It is nice to put the elements of the list of @RelatedChlidren on separate lines. This makes it easier to view and edit.
  • Regarding TextParagraphHud.TextParagraphSpacingFieldsContainer. As far as I see you have a fixed number of components(four of them). So don't use a list, but instead make four AutoPropertys for the separate components.
  • The note about setup/create methods applies here as well as for the TextParagraphPropertiesContainer
  • The "spacingIcon" in that class should also be set through the skin.
  • I think that the four elements should set the layout manager themselves rather than in that big setup method of the containing panel.
  • The way the models of the ComboBoxes are filled is not nice because every time it creates a new list with values(i.e. between 1 and 20). Instead these lists should be put into constants and probably created as Arrays.asList(1, 2, 3, 4, 5). This also is long but it is much better.
  • Also the handling should not be done by overriding the submitData method but by using LogicR3 operations.
  • Generally the same remarks hold for the TextFontHud - submitData, swingComponent.setup, validation
  • There is no @author stated in the JavaDoc of the inner classes in TextFontHud
  • The JavaDoc about italic, bold, underline and strikethrough says these are checkboxes and they no longer are.
  • In the StyleButtonsContainer.swingComponent.setupElements there are two problems. First in the if statement use instaceof operator instead of such comparison of classes. Second I dislike the idea in general. I'd rather add an aditional component that is of the given size to the list of elements and not hack the logic in such a way.
  • Please make sure that these get fixed for the implementation.

comment:11 Changed 10 years ago by dido

  • Status changed from s2c_design_ok to s3a_implementation_started
  • Imp._owners set to dido

comment:12 Changed 10 years ago by dido

  • Cc dido added
  • Status changed from s3a_implementation_started to s3b_implementation_finished
  • There is a problem with the space under the bottom control of the paragraph hud. I and deyan think that there should be a little more space below it. - This is fixed with some properties changed inside TextParagraphSpacingFieldsContainer (constraint parameters for the spring layout),however there are some minor things that should be fixed in the near future. (They are not fixed yet)
    • The font family combo box is one pixel right and not aligned with the left indent field
    • The fonst size combo box is one pixel left and not aligned with the right intent field
    • For some undefined reason the TextFontHud is way to long. There should be 5px empty area on the bottom.
  • There is commented code in HudDialog in the swingComponent property. Please make sure that the code is clean of unneeded commented out code. - All commented code is removed.
  • Write the JavaDoc properly - first the comment, then an empty line and then the author TextParagraphHud.TextParagraphPropertiesContainer, TextFontHud.FontPropertiesContainer. - Changed so it fits the requirements.
  • Add yourself as an author(with a new @author on a new line) to the TextParagraphHud, as you changed it alot. - Done.
  • The setup method of the TextParagraphHud.TextParagraphPropertiesContainer,.swingComponent() property is useless this way as it doesn't depend ot pro-lib. Its contents better fit the create method. I suppose that the revalidate and repaint calls are not needed when you change the layout manager. Also it is not nice to set the visibility together with adding the controls as this would make lots of unnecessary computation. - The functionality from the setup method is moved inside create method. Same goes for TextFontHud.
  • Don't put comma in the end of the list in the @RelatedChildren annotation as this will not compile with sun java compiler and only with the eclipse one. - Gone through the code and removed it in all cases.
  • I think that it is better to put some title in the skin for TextParagraphHud.TextParagraphPropertiesContainer.TextParagraphLineSpacing instead of just an empty string. Otherwise just don't put anything. - Now it doesn'e put anything
  • Dont "return new BoundValidation(Status.OK, "OK");", but use "return BoundValidation.SUCCESS;" instead - this is in the same class as the previous bullet. And for all other combo boxes. - Fixed inside the changed code.
  • It is nice to put the elements of the list of @RelatedChlidren on separate lines. This makes it easier to view and edit. - Done.
  • Regarding TextParagraphHud.TextParagraphSpacingFieldsContainer. As far as I see you have a fixed number of components(four of them). So don't use a list, but instead make four AutoPropertys for the separate components. - Now we are using AutoPropertys and add them manually.
  • The note about setup/create methods applies here as well as for the TextParagraphPropertiesContainer - Done.
  • The "spacingIcon" in that class should also be set through the skin. - It is in the skin now. New skin property was added inside TextParagraphSpacingFieldsContainer and also new resource property that contain an empty JLabel with placed icon inside. We may consider some other holder for the image in future.
  • I think that the four elements should set the layout manager themselves rather than in that big setup method of the containing panel. - There are two new methods added inside the BoundControl class. The first one computes the horizontal gap for the flow layout method and second for the vertical gap. Instead of setting the layout all we need to do in this point is to overwrite the methods inside the children, when we need to.
  • The way the models of the ComboBoxes are filled is not nice because every time it creates a new list with values(i.e. between 1 and 20). Instead these lists should be put into constants and probably created as Arrays.asList(1, 2, 3, 4, 5). This also is long but it is much better. - Done. However I'm not sure if we should have only round numbers between 1 and 20 for this values and why in this case we use Float type. Need to reconsider the abillity for the user to type his own values, as well as providing new default ones - that could be more useful. Font size for example may skip some numbers as 11 13 etc.
  • Also the handling should not be done by overriding the submitData method but by using LogicR3 operations.- Fixed. Inside TextFontHud new method was implemented HudLogic in order to fill extensions the TextFuncModule was updated.
  • Generally the same remarks hold for the TextFontHud - submitData, swingComponent.setup, validation-Fixed.
  • There is no @author stated in the JavaDoc of the inner classes in TextFontHud - Fixed.
  • The JavaDoc about italic, bold, underline and strikethrough says these are checkboxes and they no longer are. - Fixed.
  • In the StyleButtonsContainer.swingComponent.setupElements there are two problems. First in the if statement use instaceof operator instead of such comparison of classes. Second I dislike the idea in general. I'd rather add an aditional component that is of the given size to the list of elements and not hack the logic in such a way. - Now we are using instanceof operator, but we still use Rigid Area as it is the lowest component for Spacing issues. We may consider to use glue component in order to allow layout consistency during hud resizing.

comment:13 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 [8963]
  • Empty setup methods in resource properties.
  • Setting up visibility in the same methods as the one that adds child controls is SLOW.
  • Tooltips don't end with a dot.
  • You didn't make TextParagraphLineSpacing use logics instead of overriding submitData method.
  • When you add a TODO/FIXME/XXX comment please put simething like " --dido@2010-04-06" at the end of the line.
  • But I like the progress since the design alot

comment:14 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.