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

[Widgets] Implemented Div widget. #188

Closed
xuqingkuang wants to merge 1 commit into
intel:masterfrom
xuqingkuang:div
Closed

[Widgets] Implemented Div widget. #188
xuqingkuang wants to merge 1 commit into
intel:masterfrom
xuqingkuang:div

Conversation

@xuqingkuang

Copy link
Copy Markdown
Contributor

Limitions:

  1. The div only allow in Content.
  2. Div is not allow to be nesting.

@zhizhangchen

Copy link
Copy Markdown
Contributor

"Limitions" should be "Limitations"

"The div only allow" should be "Div is only allowed"
"Div is not allow to be nesting" should be "Nesting of Div is not allowed"

Comment thread src/css/images/widgets/jqm_div.svg Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

"jqm_footer_title" should be changed to "jqm_div_title"

@xuqingkuang

Copy link
Copy Markdown
Contributor Author

Code updated, please review.

@xuqingkuang

Copy link
Copy Markdown
Contributor Author

The code updated, the widget moved to 'Content Formatting' group, please review.

This was referenced Aug 8, 2012
@grgustaf

Copy link
Copy Markdown
Contributor

The palette icon should be in the style of "Form", not the style of "Footer". Please redo that.

Div should be categorized under "Simple HTML", not jQuery Mobile for widget set.

Other than that, it seems to work well, so we can merge that soon. Thanks.

@grgustaf

Copy link
Copy Markdown
Contributor

Could you explain why it's only allowed in Content? Did you run into problems allowing Div inside Div, or other widgets like Grid, Collapsible, etc.?

@grgustaf

Copy link
Copy Markdown
Contributor

I guess I see some weirdness if I drop a Button Group into a Div as the first element. The header bars appear right adjacent to each other vertically. On the other hand, if we put a Grid inside of a Grid or something, there is a little margin which makes it easier to use.

We should probably have a few pixels of margin like that.

Also, this made me notice that when you hover over a ButtonGroup inside a Div, both the Div and ButtonGroup header bars are highlighted (turn gray). The same is true for a Grid inside a Grid, etc; I just never noticed. Probably it would be better to ensure that only the innermost container is highlighted. That is another bug/feature I'll plan to file at some point.

@xuqingkuang

Copy link
Copy Markdown
Contributor Author

@grgustaf

Could you explain why it's only allowed in Content?

Nesting of containable widgets still have some drag and drop issues, we had submitted two patches for solve Collapsible nesting issue, but they are workaround so far.

John is investigating the issue and seems close to solve it, after the issue solved, I will upload a new patch for the widget.

The code will update for your other comments soon, thanks a lot for your reply.

@xuqingkuang

Copy link
Copy Markdown
Contributor Author

@zhizhangchen The code updated with the comment by @grgustaf the drag and drop in nesting widgets issue seems not happen in the Div widget, please review.

@zhizhangchen

Copy link
Copy Markdown
Contributor

@grgustaf Div should be categorized under "Simple HTML", not jQuery Mobile for widget set.

Limitations:
1. Div is only allowed in Content.
2. Nesting of Div is not allowed.
@xuqingkuang

Copy link
Copy Markdown
Contributor Author

@zhizhangchen My mistake, the code updated, please review.

Thank you for your reminder. :-)

@zhizhangchen

Copy link
Copy Markdown
Contributor

You didn't remove "Div" from content formatting group

@xuqingkuang

Copy link
Copy Markdown
Contributor Author

@zhizhangchen "Functional groups" and "Widget sets" are different group, maybe Div needs exist in these groups, could you confirm it's need to remove Div from content formatting ?

@grgustaf

Copy link
Copy Markdown
Contributor

Merged.

@grgustaf grgustaf closed this Aug 20, 2012
@grgustaf

Copy link
Copy Markdown
Contributor

Oops, I lost my other comment: This is a really great feature, and a nice clean patch. Thank you!

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