Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wall Stream Layout Migration #71 #74

Merged
merged 11 commits into from
Oct 30, 2020
Merged

Conversation

yurabakhtin
Copy link
Contributor

Issues: #71 and humhub/tasks#30

Copy link
Contributor

@luke- luke- left a comment

Choose a reason for hiding this comment

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

Looks very good for me. Just a few little things with the create form.

@@ -11,9 +11,16 @@

?>

<div class="contentForm_options" data-content-component="polls.Poll" style="margin-bottom:10px;margin-top:0">
<?= Html::activeTextInput($model,'question', [
'placeholder' => Yii::t('PollsModule.widgets_views_pollForm', 'Short question...'),
Copy link
Contributor

Choose a reason for hiding this comment

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

image

image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@buddh4
Copy link
Contributor

buddh4 commented Oct 21, 2020

@yurabakhtin could you change the richtext layout to LAYOUT_INLINE according to #73

@yurabakhtin
Copy link
Contributor Author

@luke- @buddh4 Changes for new layout in commit 5c18c47:

could you change the richtext layout to LAYOUT_INLINE according to #73

preview_layout

@luke-
Copy link
Contributor

luke- commented Oct 23, 2020

For me it now works as expected.

However, from a UI point of view, I don't like the 4 text inputs placed directly below each other.
image

Maybe we could:

  • Add 1-2 more rows to the description field per Default
  • Place a grey "?" in the beginning of each "Answer" text input (similar to the trash can)
  • Add headline "Possible answers:" above the Answer section.
  • Reduce the width of the answer boxes

What do you think?

@orangiene
Copy link

hi all,
here are three suggestions for the form fields. I
06_Stream_Polls-input-fields
think the simplest version is the first one. it's your choice :)

@luke-
Copy link
Contributor

luke- commented Oct 26, 2020

@orangiene Thanks for the variants!

I don't want to change too much in this stage and also in comparison to the current design.

@yurabakhtin As first step, can you please try following points:

  • Make description multi line
  • Add blank line between Title&Description and the answer section
  • Maybe: Add title above answers
  • Maybe: Make checkboxes two column

@yurabakhtin
Copy link
Contributor Author

@luke- Please check:
new_changes_ui_polls

@buddh4
Copy link
Contributor

buddh4 commented Oct 26, 2020

Looks nice, only the margin between questions/description/answers is a bit to high. Maybe use ActiveForm::field() for question and description, which will wrap the inputs in form-group divs similar to the edit view.

@yurabakhtin
Copy link
Contributor Author

@buddh4 I tested the ActiveForm::field(), the form-group has margin-bottom: 15px; but we don't need this space when only field "Question" is visible. I.e. on page load when only "Question" is visible we should keep this field with margin:0, then on click to the "Question" field we display "Description" below with inline styles:

  • first version was margin:15px 0 for the "Description" field,
  • but @luke- wrote "Add blank line between Title&Description and the answer section" and I changed that to margin:30px 0
  • now after your comment I changed to margin:20px 0 15px and added same label style for "Answers", please check how it looks:

poll_add_edit

If we will use ActiveForm::field() for "Description" then style class form-group will adds only margin-bottom:15px but we also need margin-top:~15-30px because "Question" cannot have margin-bottom.

@buddh4
Copy link
Contributor

buddh4 commented Oct 29, 2020

@yurabakhtin So the last screenshot is the current state? I think the margin and labels look fine in this one.

@yurabakhtin
Copy link
Contributor Author

@buddh4 yes, this is current state right after my last changes in commit a8563b7

@luke- luke- merged commit ade1773 into master Oct 30, 2020
@luke- luke- deleted the enh/71-wall-stream-layout-migration branch October 30, 2020 16:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants