Skip to content
This repository was archived by the owner on Mar 14, 2020. It is now read-only.

List widgets generic multi inheritance#196

Closed
zhizhangchen wants to merge 25 commits into
intel:masterfrom
zhizhangchen:list-widgets-generic-multi-inheritance
Closed

List widgets generic multi inheritance#196
zhizhangchen wants to merge 25 commits into
intel:masterfrom
zhizhangchen:list-widgets-generic-multi-inheritance

Conversation

@zhizhangchen

Copy link
Copy Markdown
Contributor

No description provided.

@grgustaf

Copy link
Copy Markdown
Contributor

OK, I really like the progress here. I love that you collapsed the two levels of items, and the text items are still selectable. Great! Playing a bit I do find some issues though.

Text List - I drop in a text list and see these things:

  1. [P3] Select the TextList and the top item gets a blue highlight around it. The highlight goes away when the window is no longer focused, not sure what's going on there.
  2. [P2] If you click to select any TextListItem, it gets that blue highlight mentioned above. But then if you click on a Text item in a different list item, the original listitem keeps the blue highlight, which looks weird.
  3. [P1] If you click on a Text node inside the listitem and try to drag it, you end up appearing to drag the whole list item instead of just the text.
  4. [P1] If you do drag a Text node like above, and it picks up the whole listitem, and then you drop it... that list item gets into a weird state where you can no longer click to select the Text lines inside of it.
  5. [P1] Probably just more side effects of the above problem: The insertion point you see as you drag in this case looks like it will drop inside another list item. Also, after you let go, one of the list items retains a red container highlight, as if you are still dragging.
  6. [P1] You are still able to delete the Text items within the listitem, but once you do it, you are not able to drop a Text item back in. I guess a solution, at least temporarily, could be to just do the same thing you did with the Split Button... if you try to delete the second button, it gives you a warning that you can't do that.
  7. [P2] I would probably change the can't-delete text there to: "This is a required part of the list item and cannot be deleted." (In particular, 'Child' is a programming term, not something the user needs to understand.)
  8. [P2] One more thing, the broken icon is displayed in the icon list items and there is a default "Iconsrc" path specified internal to the code tree. We need the same solution here that we have done for images, based on a long discussion a few weeks ago. I believe that is: the default src should be empty, display a placeholder image in the design canvas, but show the real behavior in the preview (broken image, or whatever).
  9. [P3] The user visible string probably be "Icon Source" rather than iconsrc.
  10. [P2] Count bubbles seem a little unpredictable. I only found them on the icon list, but they should probably be a property of every list type. They should probably also default to empty instead of zero (i.e. no count bubble).

From what I can tell, functionally... we are getting really close to an acceptable solution. I really appreciate that you've stuck with this and made the needed improvements. Thank you, let's push on a little farther and get this merged.

@zhizhangchen

Copy link
Copy Markdown
Contributor Author

Fixed all the problems @grgustaf mentioned and other problems.

@grgustaf

Copy link
Copy Markdown
Contributor

I still see problems #1 and #2 that I mentioned above. Also, #9 which is a one-line fix. I haven't looked at #10 yet. #3-#8 appear to be fixed, that's great! The others are minor enough that they won't prevent me from taking this.

A new one I notice:
11. [P1] If I drag a Text input into a TextList, it drops the palette icon, and it stays there until I change the Text List or its items in some way. I can even add a few of them. It looks like this happens with most widget types.

We probably should fix that before merging. I do like the fact that the list patch seems to restore the tightness of the button groups and stuff, I don't know if that's intentional or if some of the work you did to make collapsibles sort better in accordion has been lost?

@grgustaf

Copy link
Copy Markdown
Contributor

I still haven't dug into the code at all, and I need to leave early today so I'll save that for Monday.

Looking good though. I'd be willing to take this, it would be good to have it in preview 2.

@zhizhangchen

Copy link
Copy Markdown
Contributor Author

Fixed problem 1, 2, 9 and 11. Please review.

This was referenced Aug 27, 2012
@grgustaf

Copy link
Copy Markdown
Contributor

[12] If I type a bogus name for an icon like "alert", the app hangs for a few seconds. So that's probably an issue in the filesystem area. But then the worse problem is that every time I change another property it seems to go search for the bogus file again, so it keeps getting these hangs and starts making the app unusable. To compound this, even if I know that's the problem, I can't fix it because the "Icon" field is now empty and won't let me fix the incorrect value.

Maybe we should pop up an error message when it's not found and clear the field so we won't get this problem?

@grgustaf

Copy link
Copy Markdown
Contributor

Actually I would still consider taking the patchset finally but again it's very late when I'm getting to this so I will probably leave before doing so.

@grgustaf

Copy link
Copy Markdown
Contributor

I will try to start with this tomorrow to give it a really good chance of finally going in. :)

[13] Please also move the Image widget above lists in the palette.
[14] Please move the generic stuff (list item, list divider) to the top of the List widgets in the palette.

@zhizhangchen

Copy link
Copy Markdown
Contributor Author

Did you run rib locally or on a remote server? The browser is trying to access the URL you input when rendering the image. That's why it blocks for a few seconds, which I think shouldn't be, as browser should access the image from a separate thread. Maybe it's caused by the way we generate document for layoutView.

After further testing, I found that if there are many widgets in layout view, changing any property becomes very slow. So I think this problem maybe is related to the refresh mechanism of layoutView(or other views). I think maybe it's time to change the refreshing mechanism of layout view. Of course it's after the preview 2 release:)

zhizhangchen and others added 10 commits August 29, 2012 11:15
Also set the default value of count bubble to empty, i.e, no count
bubble
Sorting is still OK without this change and it causes bad
appearance of ButtonGroup and ThumbnailSplitList
Previous algorithm only supports one zone containers or Headers,
and it works hard to determine the position for insertion. The new
algorithm firstly tries to insert to the zone of the nearest
sibling using enhenced insertChildAfter/Before, if fails, uses
addChild to automatically find a zone to insert.
This was referenced Aug 29, 2012
@grgustaf

Copy link
Copy Markdown
Contributor

Because of the url-uploadable problem, I am committing this to a "lists" branch upstream, which I will plan to keep synced with master. Please try to fix that tonight and I'll be able to merge this. Maybe i'll do so anyway. :)

Also, I modified the "Icon Source" patch to use that for the property label instead of renaming iconsrc to icon.

@zhizhangchen

Copy link
Copy Markdown
Contributor Author

Fixed [object Object] problem is #274.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants