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

feat(layout): makes PF3 forms vertical #6

Merged
merged 3 commits into from Apr 17, 2019

Conversation

rvsia
Copy link
Contributor

@rvsia rvsia commented Apr 9, 2019

Description

Makes all form vertical by default.

  • removes COL component

WIP because design is not clear.

Before

image
image
image
image

After

image
image
image
image

@terezanovotna How should switch/checkboxes/radios look in a vertical form?

cc @martinpovolny @Hyperkid123

@terezanovotna
Copy link

First of all,
awesome job on always including before and after screenshots, thank you for that!

1)Text boxes 👍
2)Switched 👍
3)Checkboxes and radio buttons, let's put each item on new row, which means:

Title text
checkbox text
checkbox text

Title text
Radio text
radio text

@Hyperkid123
Copy link
Member

Title text
checkbox text
checkbox text

@terezanovotna how do we deal with single checkbox?

right now its like this:
label checkbox

@rvsia
Copy link
Contributor Author

rvsia commented Apr 10, 2019

@terezanovotna thanks for answering! 🏆

One more thing: what about checkboxes which have only one text? 😕

edit: Oh, I see Martin is faster! 🚤

@terezanovotna
Copy link

that's right. I would switch an order.

Title (if there is one)
checkbox label

@Hyperkid123 Hyperkid123 force-pushed the master branch 3 times, most recently from 445d855 to 1bc6027 Compare April 11, 2019 11:19
@rvsia
Copy link
Contributor Author

rvsia commented Apr 12, 2019

@terezanovotna

image

@rvsia rvsia force-pushed the vertical-form branch 2 times, most recently from 1f666db to a82d155 Compare April 12, 2019 07:22
@codecov-io
Copy link

Codecov Report

Merging #6 into master will not change coverage.
The diff coverage is 100%.

@@           Coverage Diff           @@
##           master       #6   +/-   ##
=======================================
  Coverage   86.31%   86.31%           
=======================================
  Files          63       63           
  Lines        1030     1030           
=======================================
  Hits          889      889           
  Misses        141      141

@rvsia
Copy link
Contributor Author

rvsia commented Apr 12, 2019

Changes in ManageIQ

Before screenshots could be found in PRs:
Import/Export ManageIQ/manageiq-ui-classic#5180
Catalog ManageIQ/manageiq-ui-classic#5139
Flavor ManageIQ/manageiq-ui-classic#5055
Tenant ManageIQ/manageiq-ui-classic#4895
Ownership ManageIQ/manageiq-ui-classic#5046
Server Relatinship ManageIQ/manageiq-ui-classic#5233
Services edit ManageIQ/manageiq-ui-classic#4985

Screenshot from 2019-04-12 09-59-06
Screenshot from 2019-04-12 09-58-23
Screenshot from 2019-04-12 09-56-32
Screenshot from 2019-04-12 09-54-23
Screenshot from 2019-04-12 09-51-02
Screenshot from 2019-04-12 09-50-14
Screenshot from 2019-04-12 09-49-43

@terezanovotna @martinpovolny @Hyperkid123

The only ugly change is in the Import/Export form, where it could be fix by just setting a different column size of the text.

@Hyperkid123
Copy link
Member

Hyperkid123 commented Apr 12, 2019

@rvsia you want the label to be multi line?

Edit: oh never mind. We can fix the alignment in MIQ directly and align it to the left. That is probably correct place right? @terezanovotna

@terezanovotna
Copy link

not sure I understand @rvsia. Are you talking about this export and import?
Screen Shot 2019-04-12 at 13 28 01

If so, I would make these buttons. with the text that's in the explanation. thoughts?

@rvsia
Copy link
Contributor Author

rvsia commented Apr 12, 2019

@terezanovotna No, about this text:

image

It is now aligned to match horizontal inputs and I just wanted to say that it is easy to align it to the left side, so you should not worry about that. 😃

@terezanovotna
Copy link

@miq-bot add_label ux/approved

@rvsia rvsia changed the title [WIP] fix(layout): makes forms vertical feat(layout): makes PF3 forms vertical Apr 16, 2019
@rvsia
Copy link
Contributor Author

rvsia commented Apr 16, 2019

@Hyperkid123 please review 🤙

@Hyperkid123
Copy link
Member

@rvsia travis is dead

Copy link
Member

@Hyperkid123 Hyperkid123 left a comment

Choose a reason for hiding this comment

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

Looks good

@Hyperkid123
Copy link
Member

@rvsia travis died again

@rvsia
Copy link
Contributor Author

rvsia commented Apr 17, 2019

@Hyperkid123 Should be OK now, dependencies are changing too fast! 🐎

@Hyperkid123 Hyperkid123 merged commit b0d6ee0 into data-driven-forms:master Apr 17, 2019
@martinpovolny
Copy link
Contributor

Please, ping also @epwinchell in these design and UX related PRs and discussions.

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.

None yet

5 participants