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

Make textarea based item selector responsive #1376

Merged
merged 2 commits into from
May 31, 2017
Merged

Make textarea based item selector responsive #1376

merged 2 commits into from
May 31, 2017

Conversation

epwinchell
Copy link
Contributor

@epwinchell epwinchell commented May 17, 2017

This PR redesigns the textarea-based "item selector" to be fully responsive, including converting png images to font icons.

#1314
https://www.pivotaltracker.com/n/projects/1613907/stories/123185601
https://bugzilla.redhat.com/show_bug.cgi?id=1374761

Old

(full desktop)
screen shot 2017-05-17 at 4 19 05 pm

(mobile)
screen shot 2017-05-17 at 4 19 16 pm

New
(full desktop)
screen shot 2017-05-17 at 4 17 09 pm

(mobile)
screen shot 2017-05-17 at 4 17 34 pm

@epwinchell
Copy link
Contributor Author

@miq-bot add_label, formatting/styling, bug, enhancement, fine/no, euwe/no, wip

@miq-bot
Copy link
Member

miq-bot commented May 17, 2017

@epwinchell Cannot apply the following label because they are not recognized:

@miq-bot miq-bot changed the title make textarea based item selector responsive [WIP] make textarea based item selector responsive May 17, 2017
@epwinchell
Copy link
Contributor Author

@miq-bot assign @dclarizio

@epwinchell
Copy link
Contributor Author

@dclarizio Please test on "edit policy's condition assignment" screen.

@epwinchell
Copy link
Contributor Author

cc @quarckster

@epwinchell
Copy link
Contributor Author

@miq-bot rm_label fine/no, euwe/no

@epwinchell
Copy link
Contributor Author

@miq-bot add_label fine/yes, euwe/yes

@epwinchell
Copy link
Contributor Author

@miq-bot assign @h-kataria

@miq-bot miq-bot assigned h-kataria and unassigned dclarizio May 24, 2017
@miq-bot
Copy link
Member

miq-bot commented May 25, 2017

Checked commits https://github.com/epwinchell/manageiq-ui-classic/compare/26cbaa179419022451dfc406a58ac7b56a4d416c~...9cf0eed7e61e8203607136f2b607c5d0b8ba3acd with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
7 files checked, 18 offenses detected

app/views/catalog/_column_lists.html.haml

  • ⚠️ - Line 16 - Use @edit[:new][:available_fields].length.zero? instead of @edit[:new][:available_fields].length == 0.
  • ⚠️ - Line 16 - Use empty? instead of length == 0.
  • ⚠️ - Line 31 - Use @edit[:new][:fields].length.zero? instead of @edit[:new][:fields].length == 0.
  • ⚠️ - Line 31 - Use empty? instead of length == 0.

app/views/miq_policy/_action_options.html.haml

  • ⚠️ - Line 155 - Line is too long. [163/160]

app/views/miq_policy/_alert_profile_details.html.haml

  • ⚠️ - Line 61 - Line is too long. [165/160]

app/views/miq_policy/_event_details.html.haml

  • ⚠️ - Line 147 - Line is too long. [173/160]
  • ⚠️ - Line 189 - Avoid more than 3 levels of block nesting.
  • ⚠️ - Line 190 - The = symbol should have one space separating it from code
  • ⚠️ - Line 52 - Line is too long. [171/160]
  • ⚠️ - Line 93 - Avoid more than 3 levels of block nesting.
  • ⚠️ - Line 94 - The = symbol should have one space separating it from code

app/views/miq_policy/_policy_details.html.haml

  • ⚠️ - Line 95 - Line is too long. [170/160]

app/views/miq_policy/_profile_details.html.haml

  • ⚠️ - Line 45 - Line is too long. [165/160]

app/views/shared/buttons/_column_lists.html.haml

  • ⚠️ - Line 16 - Use @edit[:new][:available_fields].length.zero? instead of @edit[:new][:available_fields].length == 0.
  • ⚠️ - Line 16 - Use empty? instead of length == 0.
  • ⚠️ - Line 31 - Use @edit[:new][:fields].length.zero? instead of @edit[:new][:fields].length == 0.
  • ⚠️ - Line 31 - Use empty? instead of length == 0.

@epwinchell
Copy link
Contributor Author

@miq-bot rm_label wip

@miq-bot miq-bot changed the title [WIP] make textarea based item selector responsive make textarea based item selector responsive May 30, 2017
@miq-bot miq-bot removed the wip label May 30, 2017
@epwinchell
Copy link
Contributor Author

@miq-bot assign @dclarizio

simaishi pushed a commit that referenced this pull request Jun 9, 2017
make textarea based item selector responsive
(cherry picked from commit c6e3ae2)

https://bugzilla.redhat.com/show_bug.cgi?id=1460387
@simaishi
Copy link
Contributor

simaishi commented Jun 9, 2017

Fine backport details:

$ git log -1
commit cca8238e89105b2481dd3ed8075cd83d00f86952
Author: Dan Clarizio <[email protected]>
Date:   Wed May 31 09:16:32 2017 -0700

    Merge pull request #1376 from epwinchell/responsive_selector
    
    make textarea based item selector responsive
    (cherry picked from commit c6e3ae2c7f0624cc4f45ad33be39ba036f321064)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1460387

@simaishi
Copy link
Contributor

Backported to Euwe via ManageIQ/manageiq#15322

:style => "overflow-x: scroll;",
:size => 8,
:id => "available_fields")
- if x_active_tree == :ab_tree
Copy link

Choose a reason for hiding this comment

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

Hi Eric @epwinchell , I have a question: why do we have here this condition if x_active_tree == :ab_tree twice? Once in https://github.com/ManageIQ/manageiq-ui-classic/pull/1376/files#diff-fb2d45f8fceb60d60e1267c1d866e824R2, the second time in https://github.com/ManageIQ/manageiq-ui-classic/pull/1376/files#diff-fb2d45f8fceb60d60e1267c1d866e824R12
Is there any reason for that? Thank you! :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@h-kataria Any ideas?

Copy link
Contributor

Choose a reason for hiding this comment

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

@epwinchell @hstastna if you look closely at the old/refactored code that was removed had piece of code that was not part of if block
https://github.com/ManageIQ/manageiq-ui-classic/pull/1376/files#diff-fb2d45f8fceb60d60e1267c1d866e824L8

looks like same for the second if block as well https://github.com/ManageIQ/manageiq-ui-classic/pull/1376/files#diff-fb2d45f8fceb60d60e1267c1d866e824L47

@miq-bot
Copy link
Member

miq-bot commented May 2, 2018

@epwinchell Cannot apply the following label because they are not recognized:

@miq-bot miq-bot changed the title make textarea based item selector responsive [WIP] make textarea based item selector responsive May 2, 2018
@miq-bot miq-bot assigned h-kataria and unassigned dclarizio May 2, 2018
@miq-bot miq-bot removed the wip label May 2, 2018
@miq-bot miq-bot assigned dclarizio and unassigned h-kataria May 2, 2018
@dclarizio dclarizio changed the title [WIP] make textarea based item selector responsive Make textarea based item selector responsive May 2, 2018
hstastna pushed a commit to hstastna/manageiq-ui-classic that referenced this pull request May 4, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants