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

Polls creation form #5100

Merged
merged 1 commit into from
Nov 11, 2021
Merged

Polls creation form #5100

merged 1 commit into from
Nov 11, 2021

Conversation

stefanceriu
Copy link
Member

Designs

https://www.figma.com/file/tepA3ELWBE5v4O2WB8PqWn/Polls?node-id=247%3A56744

Tickets

https://element-io.atlassian.net/browse/PSF-424

  • As a user with permission to send a message in a room
  • I can interact with a poll button in the composer
  • So that I can open the poll creation window

https://element-io.atlassian.net/browse/PSF-317

  • As a user creating a poll
  • I can enter a question
  • So that I can show respondents the question they need to answer

https://element-io.atlassian.net/browse/PSF-318

  • As a user creating a poll
  • I can enter from 2 to 20 (inclusive) responses
  • So that I can give respondents options to select from
  • To enforce the minimum of 2 requirement, the ‘create poll’ CTA is disabled unless there are at least two response options with text entered in them.
  • With respect to the above - options 1 and 2 do not have any special status that prevents them from being deleted; it is possible to add options 3 and 4, enter text in those two, then delete 1 and 2 and the CTA will still be enabled. It is also possible to delete options 1 and 2 (i.e going down to 0 choices), and then re-add them.
  • To enforce the maximum of 20 requirement, the ‘add option’ button is disabled if there are 20 options added (even if not all of them have text in). 
  • Option numbering is re-generated whenever an option is deleted. E.g. if options 1 and 2 are deleted, options 3 and 4 become 1 and 2 respectively. It is not possible to have a gap in numbering.
  • If options are deleted to bring the total to 19 or fewer, the ‘add option’ button is re-enabled.
  • If an option is left with no text entered at the point that the ‘create poll’ CTA is clicked, it is treated as if it had been deleted.

How it looks like

Screenshot 2021-11-05 at 14 35 49

Screenshot 2021-11-05 at 14 36 04

Screenshot 2021-11-05 at 14 36 13

Screenshot 2021-11-05 at 14 36 33

Screenshot 2021-11-05 at 14 36 40

Screenshot 2021-11-05 at 14 36 48

@github-actions
Copy link

github-actions bot commented Nov 5, 2021

📱 Scan the QR code below to install the build for this PR.
🔒 This build is for internal testing purpose. Only devices listed in the ad-hoc provisioning profile can install Element Alpha.

QR code

If you can't scan the QR code you can install the build via this link: https://i.diawi.com/UeJCkB

Podfile Outdated Show resolved Hide resolved
Copy link

@gaelledel gaelledel left a comment

Choose a reason for hiding this comment

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

A few a few remarks

  • For each field on focus, we need to make sure the text box is above the composer
    IMG_5384

  • The text style of titles do not match Figma. Do we have something bolder than that?

  • We need more breathing space between the Poll question/topic section and the Create options section

  • Can you check we have at least 24px between the text box and "add option" button?

Copy link
Member

@langleyd langleyd left a comment

Choose a reason for hiding this comment

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

Really great stuff here 🔥. Just a few questions/comments.

Config/BuildSettings.swift Outdated Show resolved Hide resolved
RiotSwiftUI/Modules/Common/Util/MultilineTextField.swift Outdated Show resolved Hide resolved
RiotSwiftUI/Modules/Common/Util/MultilineTextField.swift Outdated Show resolved Hide resolved
@stefanceriu
Copy link
Member Author

Seeing as customizing the automatic keyboard avoidance on the textFields is complicated we've agreed to tackle it later, after implement more core functionality.

Copy link

@gaelledel gaelledel left a comment

Choose a reason for hiding this comment

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

Approved given we will tackle the bug with text fields not appearing above the composer when active at later stage.
Thanks!

Copy link
Member

@langleyd langleyd left a comment

Choose a reason for hiding this comment

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

Just one comment regarding the ViewModifier but LGTM otherwise.

@stefanceriu stefanceriu force-pushed the stefan/polls branch 2 times, most recently from 9a662d9 to eca7a7e Compare November 11, 2021 08:45
- added input toolbar poll creation action.
- reordered input toolbar actions as per designs.
- added multiline text field and extracted common components.
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.

3 participants