Skip to content
This repository has been archived by the owner on Aug 25, 2021. It is now read-only.

View button responsiveness fix #158

Merged
merged 1 commit into from
Nov 14, 2019
Merged

View button responsiveness fix #158

merged 1 commit into from
Nov 14, 2019

Conversation

epwinchell
Copy link
Contributor

@epwinchell epwinchell commented Oct 29, 2019

The PR adds in the missing "toolbar-pf-action-right" class div, which makes the view buttons align left when in the responsive state.

Before (demo)
Screen Shot 2019-10-29 at 11 35 43 AM

After (demo)
Screen Shot 2019-10-29 at 11 29 29 AM

Before (ui classic)
Screen Shot 2019-10-29 at 11 34 25 AM

After (ui classic)
Screen Shot 2019-10-29 at 11 30 44 AM

@epwinchell
Copy link
Contributor Author

@miq-bot assign @martinpovolny

@epwinchell
Copy link
Contributor Author

@miq-bot bug, cleanup

@miq-bot
Copy link
Member

miq-bot commented Oct 29, 2019

@epwinchell unrecognized command 'bug', ignoring...

Accepted commands are: add_label, add_reviewer, assign, close_issue, move_issue, remove_label, rm_label, remove_reviewer, set_milestone

@miq-bot
Copy link
Member

miq-bot commented Oct 29, 2019

Checked commit https://github.com/epwinchell/react-ui-components/commit/e42d78292126bc1b535ef0d54d62dd295ff757b6 with ruby 2.4.6, rubocop 0.69.0, haml-lint 0.20.0, and yamllint 1.10.0
0 files checked, 0 offenses detected
Everything looks fine. 👍

@martinpovolny martinpovolny added the bug Something isn't working label Nov 14, 2019
@martinpovolny martinpovolny merged commit 1718e78 into ManageIQ:master Nov 14, 2019
@karelhala
Copy link
Contributor

🎉 This PR is included in version 0.11.56 🎉

The release is available on:

Your semantic-release bot 📦🚀

@himdel
Copy link
Contributor

himdel commented Nov 14, 2019

@martinpovolny We really need to fix the jest bits, this also breaks ui travis :)

 FAIL  app/javascript/spec/toolbar/miq-toolbar.spec.jsx
  ● <MiqToolbar /> › renders ok for generic case
    expect(received).toMatchSnapshot()
    Snapshot name: `<MiqToolbar /> renders ok for generic case 1`
    - Snapshot
    + Received
    @@ -86,11 +86,15 @@
            <ToolbarView
              onClick={[Function]}
              views={Array []}
            >
              <div
    -           className="toolbar-pf-view-selector pull-right form-group"
    -         />
    +           className="toolbar-pf-action-right"
    +         >
    +           <div
    +             className="form-group toolbar-pf-view-selector"
    +           />
    +         </div>
            </ToolbarView>
          </div>
        </Toolbar>
      </MiqToolbar>

(regenerating the snapshot for now - ManageIQ/manageiq-ui-classic#6418)

@martinpovolny
Copy link
Member

Oh you fixed it already.

@martinpovolny
Copy link
Member

The problem here is that I have to get the change merged to get the bot do a release.

After that, I can update the UI classic.

But at that point the test is already broken.

I think that what is needed here is remove the snapshot test from the UI classic (or move it here) and do a different type of test (w/o a snapshot) on UI classic side.

WDYT?

@martinpovolny
Copy link
Member

Btw, on top of thanking @himdel for quickly fixing this I also thank @Fryguy for quickly merging the fix ;-)

@himdel
Copy link
Contributor

himdel commented Nov 18, 2019

Yup, either we should think about some way of getting the react-ui-components release (and ui-components) trigger a push to ui-classic (and ui-service) master, to use the new version right away,

or we can't do snapshot tests across repo boundaries.

Or both :).

I think there should be a way of telling jest "these are our components, expand them by default, but those are from elsewhere, stop at those".

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working released
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants