Fix Button spacing issues and add horizontal scroll view in case actions orientation is horizontal . #1452
Conversation
|
|
| @@ -255,6 +260,15 @@ else if (alignment == ActionAlignment.Center.swigValue()) | |||
| ActionElementRenderer.getInstance().render(renderedCard, context, fragmentManager, actionButtonsLayout, actionElement, cardActionHandler, hostConfig); | |||
| } | |||
|
|
|||
| if (viewGroup != null && hostConfig.getActions().getActionsOrientation().swigValue() == ActionsOrientation.Horizontal.swigValue()) | |||
| { | |||
There was a problem hiding this comment.
We are adding actionbuttonslayout above and then removing it from view group. Can you change the code above to selectively add to scroll view if required and avoid the add and removal
There was a problem hiding this comment.
@rednivrug15 I agree with this comment, can you change it?
There was a problem hiding this comment.
@almedina-ms currently the show card action has a very tight binding to the view hierarchy . It expects the parent's parent ( view.getParent().getParent() ) to be top level card . We will have to change the show card layout's implementation if we do not want to do it this way .
There was a problem hiding this comment.
@almedina-ms Since the horizontal scroll has been implemented I will revert the change for horizontal scroll and keep rest of the bug fixes here . Can you please look into this ?
| return (String) getSpinner().getSelectedItem(); | ||
| int index = getSpinner().getSelectedItemPosition(); | ||
| String selectedItem = ""; | ||
| if (index >= 0) |
There was a problem hiding this comment.
nit: check index < getchoices().length just to be super safe?
| String selectedItem = ""; | ||
| if (index >= 0) | ||
| { | ||
| selectedItem = choiceSetInput.GetChoices().get(index).GetValue(); |
There was a problem hiding this comment.
Nit: Simply return here.. why store?
There was a problem hiding this comment.
I would prefer having one single return point in this case, after all it's just one extra line of code
There was a problem hiding this comment.
Not needed as the SDK prefers single point of return .
| @@ -242,6 +244,9 @@ else if (alignment == ActionAlignment.Center.swigValue()) | |||
| actionButtonsLayout.setOrientation(LinearLayout.HORIZONTAL); | |||
| } | |||
|
|
|||
| Spacing spacing = hostConfig.getActions().getSpacing(); | |||
| BaseCardElementRenderer.setSpacingAndSeparator(context, viewGroup, spacing, false, hostConfig, true); | |||
There was a problem hiding this comment.
Can you add a comment next to the false and true values to know what do they stand for?
There was a problem hiding this comment.
Yes. Will add the same comment as has been used elsewhere .
…ons orientation is horizontal . (#1452) * fix button spacing bugs and add horizontal scroll view to buttons * fix blank line * add scrollview only if actions alignment is horizontal * add comments * added check to see if selected item is within range * improve fact set renderer * remove unnecessary changes
…ons orientation is horizontal . (#1452) * fix button spacing bugs and add horizontal scroll view to buttons * fix blank line * add scrollview only if actions alignment is horizontal * add comments * added check to see if selected item is within range * improve fact set renderer * remove unnecessary changes
…ons orientation is horizontal . (microsoft#1452) * fix button spacing bugs and add horizontal scroll view to buttons * fix blank line * add scrollview only if actions alignment is horizontal * add comments * added check to see if selected item is within range * improve fact set renderer * remove unnecessary changes
Fix spacing between two buttons . It should follow the hostConfig property for spacing between two buttons #1432
Fix the spacing between the buttons layout and the last card element . This should also follow the host config property #1432
Add horizontal scroll view to make buttons scrollable in case they exceed the width available #1426.
Fix a bug in multiple choice input where title is returned instead of returning the value when Action.Submit button is pressed #1449.
Fix fact set rendering issue where the content is clipped #1472