search

kwiliarty / glyphworks

MIT License
0 stars 0 forks source link

Copy button for Glyph detail pages #83

Closed kwiliarty closed 1 year ago

kwiliarty commented 1 year ago

Requires a Button component. Or maybe just an extension of the ClipboardCopy compontent.

image
andyzito commented 1 year ago

@kwiliarty Playing around with this and had a thought as I was discovering that it's easier to put the button inside of GlyphChip -- what if you moved the Card copy button there as well? I'm envisioning something like a hover state that shows a "copy" hint when you hover over the Chip, and then you can still click the rest of the card to go to the details page. Thoughts?

kwiliarty commented 1 year ago

How would it behave with a touch screen?

kwiliarty commented 1 year ago

Just out of curiosity, why is it easier to put the button inside the GlyphChip?

andyzito commented 1 year ago

Well, so, it's totally possible I'm off base on the GlyphChip thing, but what I was looking at was the scoping of the theme -- I wanted the button to match the style of the chip and it seemed like nesting them was an easy way to accomplish that without duplication. But that might be silly, my React is rusty.

Re: the card copy button, I guess in my head I was imagining that clicking the symbol to copy would be intuitive even w/o hover state to cue. But thinking back on that now I don't think I agree.

Anyway, will try to get a PR for this soon :)

kwiliarty commented 1 year ago

Your solution is clever, and not something I had thought of. I like it because it's helping me push the question back a step.

The plus side of what you're doing is that it reuses the ClipboardCopy component. The downside is that the button styling would be tied up with just this component. Maybe the crux of the matter is how we negotiate those potentially conflicting interests.

I hope you'll forgive me that the spec and the scope are evolving. For me a part of the fun on this project is taking the time to think some things through.

Maybe what we ultimately need is our own Button component and our own IconButton component. Then we could implement the ClipboardCopy component as you have done so that it can manifest itself as either of those, but not so that it is the home of the styling for either one. This makes me think that we could go for a phased approach. We could follow your path but then follow-up by extracting out the Button or TextButton and IconButton. What do you think of that idea? If we do that, I'd still like to tune the styling you've offered, but for now what do you think of that general approach?

kwiliarty commented 1 year ago

@andyzito I've just pushed some style tweaks so that 1) the positioning of the pure icon buttons is not altered from what it was and 2) the button with text is a bit lighter visually.

image

Let me know what you think.

andyzito commented 1 year ago

I have a mild personal preference for the visual similarity between red Chip and red Button, but I think overall it looks good! Let me open up a PR for this. If I don't get to the second-stage extraction of icon/button that you describe above, we can always get that on a second pass.

kwiliarty commented 1 year ago

I agree about leaving the extraction for a second pass. I'm also open to playing more with the styles. Thanks for the contribution!