Skip to content

Conversation

@dcmouyard
Copy link
Member

This PR closes #488.

This work could potentially affect button styling from Drupal core and contrib styles, so I think this needs thorough QA before it gets merged in. I’m going to try this approach on NRDC if we want to use that project as a guinea pig.

overflow: visible;
padding: 0;
text-align: left;
text-decoration: underline;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks! I switched to using that mixin to account for a bunch of the default button styles.

@coreylafferty coreylafferty marked this pull request as draft December 2, 2021 19:04
@dcmouyard dcmouyard added this to the 5.x milestone Dec 14, 2021
* 5.x: (198 commits)
  version bump
  updated with Dan's suggestions
  Optimize F1 logo
  Update README to match coding standards
  replaced missing f1 logo with local copy
  fixed generated image directory path and updated some documentation
  Remove whitespace around buttons
  Update button spacing styles
  fix title_display var name
  added front class to homepage in storybook
  Check if menubarItem has a popupMenu before closing
  Close all open dropdown menus before opening new one
  Fix required form item labels in Drupal
  Fix indentation in form item template
  version bump
  updated some packages
  Update Gesso for node 16
  version bump
  version bump
  added Prettier formatting
  ...
@coreylafferty coreylafferty marked this pull request as ready for review March 9, 2023 19:14
@coreylafferty coreylafferty changed the base branch from 5.x to 5.x-RC March 9, 2023 19:15
@kmonahan
Copy link
Collaborator

This PR is from 2021 (!!). Reviewing the issue, seems like @dcmouyard , you validated this approach on NRDC and confirmed the buttons it applied to made more sense styled as links rather than matching c-button. Confirming this one still needs review + eventual merge?

@dcmouyard
Copy link
Member Author

I still haven’t run into any problems with this approach, and I would use it again on all of my future projects.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

4 participants