Add a copy button for the UUIDs of Samples, Batches and Boxes

[diegoliberman]

Story

As a generic user of CDx, I want to be able to copy the UUID so that that I can paste it somewhere else without the need of selecting the text.

Entities for adding the UUID

1. Sample
2. Batch
3. Box

Acceptance criteria

1. I can easily copy the mentioned UUIDs.
2. I have a feedback tooltip: "Copied".

[diegoliberman]

@jkicillof please could you provide an Icon for copying the UUID? Thanks!

[jkicillof]

@diegoliberman I updated every sample, batch and box screen with UUID and copy button. The icon is from Google Material Icons set, yo can copy the icon name and use that if the icon font is installed, if not I can export it as svg. Batches already have an id but I think is manually entered, should we keep both?

[diegoliberman]

The Batch ID is manually entered, but I think it might provide value. Because when creating a box, you need to enter the Batch IDs, and perhaps it's comfortable to be able to copy and paste it.

[ysbaddaden]

@jkicillof We're using a font file that is generated from https://icomoon.io/app/#/select where we can select the material icon to be added (details).

Also, it would be nice to confirm the copy, for example a simple "copied" tooltip.

[jkicillof]

icomoon.zip
@ysbaddaden here's the updated font. Regarding the tooltip I totally agree, we should use a snackbar like the ones shown here with the new copy Icon and a title like "UUID copied" and the UUID as description.

[straight-shoota]

I'm not sure a snack bar notification is a good idea for this. Those usually notify about backend actions.

IMO notification about copy to clipboard should better be placed in close proximity to the action item, like a tool tip. See the examples on https://clipboardjs.com/, I think that makes a lot of sense for a smooth UX.

[jkicillof]

@straight-shoota ok, that makes sense. I don't know if this screen was implemented but you can find a tooltip style there.

[omelao]

@jkicillof I don't know how the copy was added in the source, but it would be nice to add it at the end, as all the codes have changed. The \e900 was alert, now it is \e901 and the copy became \e900.

[jkicillof]

I'm on it

[jkicillof]

icomoon.zip
@omelao Now it should be ok

[jkicillof]

To avoid icons get replaced if the code changes we should use the icon name instead of the code. If ligatures are enabled you could use for example local_shipping instead of \e961 to show a truck. If you open the font on Icomoon you can see every icon with its code and name.

[jkicillof]

icomoon.zip
Here's a new version with normalized names.

[omelao]

Ok, but we have to create the CSS class using the char code. As you can see here:

[omelao]

I don't know how to use ligatures, I can learn, but the whole project was already like this.

[jkicillof]

I think font-variant-ligatures are enabled by default. You should just replace content: \e961; with content: local_shipping.

[diegoliberman]

If the entire project is using the codes, but we want to use ligatures, we should at least think about a plan on how to do that:

1. Do we change all codes at once?
2. Do we start using the ligatures just for the new icons?
3. Other?

@ysbaddaden do you have any preference?

Personally, I think I would start using ligatures (if everyone agrees that's better than the codes), and I'd create a technical debt ticket for changing all codes for their corresponding label.

[jkicillof]

I updated the font and removed icon from names. I noticed that there's a demo.html in the zip file where you can check codes and names and do a font test drive at the end of the page. Ligatures are not exported by default so I had to copy the icon name and paste it in the ligatures field (we could add multiple ligatures for each icon separated by comma). If we go this way we should update this doc with an extra step like Add a snake case name and ligature for every new icon. Step 4 could be removed.
icomoon.zip

[omelao]

Ok, it's working. But, I don't know if I should update every class on the task I'm on.

[diegoliberman]

Ok, it's working. But, I don't know if I should update every class on the task I'm on.

If it's just those classes that we need to change, or if it takes less than a couple of hours, go ahead and change it right now as part of this issue.

[straight-shoota]

Before refactoring this: I think it would be worth considering to ditch the icon font and just embed SVGs directly. That's a much easier workflow going forward, no need to deal with icomoon when adding new icons.
We had briefly talked about this in slack: https://manas.slack.com/archives/C034BR64J/p1648220714998249?thread_ts=1648220295.926889&cid=C034BR64J
/cc @ysbaddaden

[ysbaddaden]

@jkicillof The only process that worked for me is the one detailed here. It's quite tedious, and error prone.

I agress with @straight-shoota: we should drop the icomoon font. Having a bunch of SVG files under app/assets/images/icons to use directly would be much simpler.

It's just a little tricky because we do take advantage of having a font to change the icons' color depending on context. We thus can't simply use them as background-image: asset-url("icons/copy.svg").

Ideally we'd have an icon(name) helper that would paste the icon as SVG right into the HTML. We can then colorize 'em with the fill CSS property. That's the proper and recommended solution, but it requires to inspect every template for icon class names to replace them with the icon helper... this is tedious.

Now, I just found out that we could use CSS filters to colorize an image... which means that we could probably use background-image with filter. See https://domysee.com/blogposts/coloring-white-images-css-filter to understand how it works. We'd merely update _sprites.css, nothing more (except for renaming it as `_icons.scss).

Here is an example:

.icon-blue {
  // rgb(238, 43, 36) => hsl(2deg, 185.6%, 53.7%)
  filter: invert(1) brightness(50%) sepia(100%) saturate(10000%)
         hue-rotate(207deg) saturate(189.7%) brightness(154.1%);
}

.icon-red {
  // rgb(33, 150, 243) => hsl(2deg, 85.6%, 53.7%)
  filter: invert(1) brightness(50%) sepia(100%) saturate(10000%)
          hue-rotate(2deg) saturate(185.6%) brightness(153.7%); 
}

Last but not least: nothing prevents us from gradually migrating to an icon(name) helper.

[jkicillof]

I don't think the filter solution is clean because finding the color requires a few transformation (hex > rgb > hsl). If you can import the svgs and then use fill on css is ok to me but you'll have to update every place where it was used. We don't add icons very often so maybe we can leave it as it is right now and replace code with ligature to avoid code changes break every icon used.

[omelao]

If I were to start something new I would certainly use the separate SVG but as the wheel is already turning this way, I think we can keep this for now. I liked the ligatures option, but I don't think it's necessary to change to ligatures, because in the icomon.zip itself comes a styles.css file that has exactly what we have in _sprites.css. If we have problems with the character codes, it would be enough to copy the contents of styles.css to _sprites.css without even needing the contents with ligatures. I calculated 100 icons to update.

[diegoliberman]

All right @omelao, I agree. Let's wrap up this ticket and create a PR when you're ready. Thanks!

[straight-shoota]

It's just a little tricky because we do take advantage of having a font to change the icons' color depending on context. We thus can't simply use them as background-image: asset-url("icons/copy.svg").

Is this a real problem, though? Where would we actually use an SVG image as background?

[straight-shoota]

as the wheel is already turning this way, I think we can keep this for now.

Sure, there's no additional cost to that. Except that adding every new icon is more tedious than it needs to be.
I think it would be quite easy to just start using SVG for new icons.

[mmuller]

Working properly in version 7757444