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

fix(pagination): minor css for react-bootstrap pagination dropdown #910

Merged
merged 1 commit into from
Jan 16, 2018

Conversation

priley86
Copy link
Member

@priley86 priley86 commented Jan 12, 2018

Description

This PR attempts to address a minor issue using the React Bootstrap DropdownButton instead of bootstrap-select. The DOM rendered is a tad bit different from bootstrap-select and customizing this via a few minor CSS tweaks seems to be the easier option.

Link to OSUX story:
https://patternfly.atlassian.net/browse/OSUX-339

PF-React PR:
patternfly/patternfly-react#143

Link to React Storybook

Note: I am ignoring the bootstrap-select paddings added to the dropdown-toggle and would prefer to just leave it at min-width:auto as to not introduce more CSS.

Changes

  • minor tweaks to pagination btn-group selector as to ensure React Bootstrap button-group is styled closely w/o the bootstrap-select class
  • ensures the React Bootstrap dropdown-menu is min-width:auto

Link to rawgit and/or image

For example: Here is a link to Alerts on rawgit

This is an image:

Image description

PR checklist (if relevant)

  • Cross browser: works in IE9
  • Cross browser: works in IE10
  • Cross browser: works in IE11
  • Cross browser: works in Edge
  • Cross browser: works in Chrome
  • Cross browser: works in Firefox
  • Cross browser: works in Safari
  • Cross browser: works in Opera
  • Responsive: works in extra small, small, medium and large view ports.
  • Preview the PR: An image or a URL for designer to preview this PR is provided.

Copy link
Member

@andresgalante andresgalante left a comment

Choose a reason for hiding this comment

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

Hi @priley86 Although I don't see a technical problem to merge this, it leads me to think how much we should clutter patternfly's core code base for the sake of one JS framework implementation.

If we start treating PF core as a dump site to all possible implementations, then everyone loses. any patternfly user that is not using our React implementation will ship more useless code if we merge this PR.

@jeff-phillips-18 I'll let you decide on this.

@dtaylor113
Copy link
Member

Hi @andresgalante, @priley86, yeah, it's a balancing act. I think in general we are trying to avoid whenever possible adding less/css to the js framework repos so that patternfly core is the main/single place for Patternlfy less/scss/css. This angular-patternfly issue/epic is an attempt to move less to core where ever possible.

@priley86
Copy link
Member Author

@andresgalante i don't see why pf-core should be scoping css specifically for jquery extensions at this point. Is there anything here that is really pf-react specific though? 🙋‍♂️

@jeff-phillips-18
Copy link
Member

@andresgalante Is the specificity that is there now required for patternfly?

@jeff-phillips-18
Copy link
Member

I see that we do need to provide the original level of specificity in order to override the bootstrap setting. I'm OK with this change since it is really just providing an alternative to using the jquery bootstrap-select.

Copy link
Member

@cdcabrera cdcabrera left a comment

Choose a reason for hiding this comment

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

The selector addition and minor resets look easy enough. Curious how an inherited float was influencing a flex layout... overall lgtm.

@jeff-phillips-18 jeff-phillips-18 merged commit 96b5f03 into patternfly:master Jan 16, 2018
priley86 referenced this pull request in ManageIQ/manageiq-v2v May 31, 2018
The css comes from patternfly/patternfly#910, wrapped in a `.miq-v2v` so that we can be sure this only affects v2v screeens.

Unfortunately dialogs get opened directly under body, so any such class needs to live on body.

(Fortunately there are no cross-controller ajax transitions in gaprindashvili.)
priley86 referenced this pull request in ManageIQ/manageiq-v2v May 31, 2018
The css comes from patternfly/patternfly#910, wrapped in a `.miq-v2v` so that we can be sure this only affects v2v screeens.

Unfortunately dialogs get opened directly under body, so any such class needs to live on body.

(Fortunately there are no cross-controller ajax transitions in gaprindashvili.)
AparnaKarve referenced this pull request in ManageIQ/manageiq-v2v Jun 1, 2018
The css comes from patternfly/patternfly#910, wrapped in a `.miq-v2v` so that we can be sure this only affects v2v screeens.

Unfortunately dialogs get opened directly under body, so any such class needs to live on body.

(Fortunately there are no cross-controller ajax transitions in gaprindashvili.)
priley86 referenced this pull request in ManageIQ/manageiq-v2v Jun 1, 2018
The css comes from patternfly/patternfly#910, wrapped in a `.miq-v2v` so that we can be sure this only affects v2v screeens.

Unfortunately dialogs get opened directly under body, so any such class needs to live on body.

(Fortunately there are no cross-controller ajax transitions in gaprindashvili.)
priley86 referenced this pull request in ManageIQ/manageiq-v2v Jun 5, 2018
The css comes from patternfly/patternfly#910, wrapped in a `.miq-v2v` so that we can be sure this only affects v2v screeens.

Unfortunately dialogs get opened directly under body, so any such class needs to live on body.

(Fortunately there are no cross-controller ajax transitions in gaprindashvili.)
AparnaKarve referenced this pull request in ManageIQ/manageiq-v2v Jun 6, 2018
The css comes from patternfly/patternfly#910, wrapped in a `.miq-v2v` so that we can be sure this only affects v2v screeens.

Unfortunately dialogs get opened directly under body, so any such class needs to live on body.

(Fortunately there are no cross-controller ajax transitions in gaprindashvili.)
AparnaKarve referenced this pull request in ManageIQ/manageiq-v2v Jun 8, 2018
The css comes from patternfly/patternfly#910, wrapped in a `.miq-v2v` so that we can be sure this only affects v2v screeens.

Unfortunately dialogs get opened directly under body, so any such class needs to live on body.

(Fortunately there are no cross-controller ajax transitions in gaprindashvili.)
priley86 referenced this pull request in ManageIQ/manageiq-v2v Jun 12, 2018
The css comes from patternfly/patternfly#910, wrapped in a `.miq-v2v` so that we can be sure this only affects v2v screeens.

Unfortunately dialogs get opened directly under body, so any such class needs to live on body.

(Fortunately there are no cross-controller ajax transitions in gaprindashvili.)
AparnaKarve referenced this pull request in ManageIQ/manageiq-v2v Jun 12, 2018
The css comes from patternfly/patternfly#910, wrapped in a `.miq-v2v` so that we can be sure this only affects v2v screeens.

Unfortunately dialogs get opened directly under body, so any such class needs to live on body.

(Fortunately there are no cross-controller ajax transitions in gaprindashvili.)
AparnaKarve referenced this pull request in ManageIQ/manageiq-v2v Jun 14, 2018
The css comes from patternfly/patternfly#910, wrapped in a `.miq-v2v` so that we can be sure this only affects v2v screeens.

Unfortunately dialogs get opened directly under body, so any such class needs to live on body.

(Fortunately there are no cross-controller ajax transitions in gaprindashvili.)
AllenBW referenced this pull request in ManageIQ/manageiq-v2v Jun 18, 2018
The css comes from patternfly/patternfly#910, wrapped in a `.miq-v2v` so that we can be sure this only affects v2v screeens.

Unfortunately dialogs get opened directly under body, so any such class needs to live on body.

(Fortunately there are no cross-controller ajax transitions in gaprindashvili.)
AparnaKarve referenced this pull request in ManageIQ/manageiq-v2v Jun 19, 2018
The css comes from patternfly/patternfly#910, wrapped in a `.miq-v2v` so that we can be sure this only affects v2v screeens.

Unfortunately dialogs get opened directly under body, so any such class needs to live on body.

(Fortunately there are no cross-controller ajax transitions in gaprindashvili.)
mzazrivec referenced this pull request in mzazrivec/miq_v2v_ui_plugin Jul 9, 2018
The css comes from patternfly/patternfly#910, wrapped in a `.miq-v2v` so that we can be sure this only affects v2v screeens.

Unfortunately dialogs get opened directly under body, so any such class needs to live on body.

(Fortunately there are no cross-controller ajax transitions in gaprindashvili.)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants