search

patternfly / patternfly-ng

The code for a set of Angular 6+ components for the PatternFly project. Note that the release/3.x branch supports Angular 4 and 5.
http://www.patternfly.org/patternfly-ng
MIT License
88 stars 55 forks source link

Update Copy Component Pattern to match PF Core #433

Closed seanforyou23 closed 6 years ago

seanforyou23 commented 6 years ago

PFNG blockcopy and inlinecopy existed before it was an official pattern in patternfly core. Now that the pattern is in place on the design side, let's update our implementation to align with the styling and interactions defined at https://www.patternfly.org/pattern-library/forms-and-controls/copy-to-clipboard/

Let's minimize the breakage of public api parts and document any necessary changes.

dlabrecq commented 6 years ago

I don't know that we need to consolidate the two components? Other than the button style, the overall design isn't all that different. That is, it appears that there are still two types; inline and block. And inline looks exactly the same.

Why not keep the two components, but add the style changes to the block component? That would help ensure we don't break the existing APIs.

seanforyou23 commented 6 years ago

Yea I actually prefer keeping both just wasn't sure that was an option. I was thinking to fulfill design all the requirements needed to exist in a single component. Between the two components we have all the bases covered, so if that's ok with everyone else, it works for me!

dlabrecq commented 6 years ago

If we were starting from scratch, one component would do. Now that we have introduced two, we can't easily change/remove the APIs. We would be expected to deprecate one (or both) components. Then, they could be removed in a major release.

seanforyou23 commented 6 years ago

Just an update, hitting an issue that seems to be related to the pfng demo app - interacting with input elements inside certain example components (like inlinecopy, and filter for example) causes the individual demo templates to perform a disappearing act. May require a tweak to the build, not quite sure yet. Will update again once we find the root cause.