Add label editor to create custom labels #740

serk12 · 2021-08-19T14:55:30Z

Label editor
fix transparent labels

Signed-off-by: Marc Prat Masó marc.prat.maso@estudiantat.upc.edu

Developer Certificate of Origin
Version 1.1

Copyright (C) 2004, 2006 The Linux Foundation and its contributors.
1 Letterman Drive
Suite D4700
San Francisco, CA, 94129

Everyone is permitted to copy and distribute verbatim copies of this
license document, but changing it is not allowed.

Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
have the right to submit it under the open source license
indicated in the file; or

(b) The contribution is based upon previous work that, to the best
of my knowledge, is covered under an appropriate open source
license and I have the right under that license to submit that
work with modifications, whether created in whole or in part
by me, under the same open source license (unless I am
permitted to submit under a different license), as indicated
in the file; or

(c) The contribution was provided directly to me by some other
person who certified (a), (b) or (c) and I have not modified
it.

(d) I understand and agree that this project and the contribution
are public and that a record of the contribution (including all
personal information I submit with it, including my sign-off) is
maintained indefinitely and may be redistributed consistent with
this project or the open source license(s) involved.

Signed-off-by: Marc Prat Masó <marc.prat.maso@estudiantat.upc.edu>

ghutchis · 2021-08-19T15:25:00Z

avogadro/qtplugins/label/labeleditor.cpp

+                 Rendering::TextProperties::VCenter);
+  tprop.setFontFamily(Rendering::TextProperties::SansSerif);
+
+  tprop.setColorRgb(255, 255, 255);


I think we definitely want to let people pick colors for the labels. (Not per-label.)

This is only in the label edit mode not the render display. But maybe white could be annoying if the background is white?

The white can be annoying for light-colored atoms (e.g., hydrogen)

ghutchis · 2021-08-19T15:27:31Z

avogadro/qtplugins/label/labeleditor.cpp

+
+void LabelEditor::save()
+{
+  m_molecule->beginMergeMode(tr("write Label"));


"Write Label" or maybe "Set Label" or "Create Label"?

+1 to "Create Label"

ghutchis · 2021-08-19T16:07:38Z

avogadro/qtplugins/label/labeleditor.cpp

+    m_selected = (clickedObject.type == Rendering::AtomType);
+    if (m_selected) {
+      m_selectedAtom = m_molecule->atom(clickedObject.index);
+      m_text = QString::fromUtf8(m_selectedAtom.label().c_str());


QString::fromStdString() should also work, I think...

ghutchis · 2021-08-19T16:08:54Z

This Ubuntu failure just seems to be a timeout. (The test set seems to be failing on bond tests #735.)

ghutchis · 2021-08-19T16:11:45Z

I'll test the UI on this.

In the previous version, if you were showing the label display, it would look like you'd put a new label on top of the label display, which was weird. I think a user would expect clicking would offer a default (e.g., the "H3" or "C1" from the label display if nothing is set) so that text could be changed.

The label engine probably needs to offer a popup menu / QComboBox with a few alternatives including "Custom Labels Only".

github-actions · 2021-08-19T16:46:03Z

Here are the build results
macOS.dmg
Win64.exe
Artifacts will only be retained for 90 days.

avo-bot · 2021-08-19T19:09:51Z

This pull request has been mentioned on Avogadro Discussion. There might be relevant details there:

https://discuss.avogadro.cc/t/atom-label-numbering-in-xyz-files/3608/3

Signed-off-by: Marc Prat Masó <marc.prat.maso@estudiantat.upc.edu>

serk12 · 2021-08-20T11:00:19Z

Something like this? I will change the checbox with the QComboBox and add the same options to the residues.

ghutchis · 2021-08-20T18:52:26Z

Yes, I think the checkboxes are types of atom labels. So you could probably have something like:

Atom Labels: (as a combo box)

None
Atom Index
Element Symbol
Symbol & Number in Group
Custom

(and "Custom" => atom.label())

(Sorry, the font is weird in Avo1 on my Mac now.)

ghutchis · 2021-08-20T18:54:15Z

We never had residue labels, but probably:

Residue Labels:

None
Residue Name
Residue ID
Residue Name + ID

I've definitely seen cases where people annotate 2-3 particular residues with labels.

github-actions · 2021-08-21T15:34:35Z

Here are the build results
macOS.dmg
Ubuntu-2004.tar.gz
Win64.exe
Artifacts will only be retained for 90 days.

ghutchis · 2021-08-22T14:31:18Z

Let me know when you think this is ready to merge. The code looks good - just trying to get the UI / UX right. 😄

Signed-off-by: Marc Prat Masó <marc.prat.maso@estudiantat.upc.edu>

github-actions · 2021-08-23T13:42:54Z

Here are the build results
macOS.dmg
Ubuntu-2004.tar.gz
Win64.exe
Artifacts will only be retained for 90 days.

ghutchis · 2021-08-23T15:49:30Z

avogadro/qtplugins/label/label.cpp

+                       SLOT(residueLabelType(int)));
+      index = residue->findData(int(residueOptions));
+      residue->model()->sort(0, Qt::AscendingOrder);
+      form->addRow(QObject::tr("residue label:"), residue);


tr("Residue Label:")

ghutchis · 2021-08-23T15:49:56Z

avogadro/qtplugins/label/label.cpp

+                       SLOT(atomLabelType(int)));
+      int index = atom->findData(int(atomOptions));
+      atom->model()->sort(0, Qt::AscendingOrder);
+      form->addRow(QObject::tr("Atom label:"), atom);


tr("Atom Label:")

ghutchis · 2021-08-23T16:01:48Z

I think there are two minor issues with the latest version.

I don't think we need to offer so many options - many users are students. For now, I think I'd stick to the options I mentioned above (Add label editor to create custom labels #740 (comment)). It's obviously easy to add more, but I suspect these are the most common.
For translation / i18n doing programmatic assembly is bad. European languages work like tr("Element") + tr("Index") but it's much better to translate each assembled string as a literal. (I learned this lesson the hard way .. one benefit of 25+ translations of Avo1.)

ghutchis · 2021-08-23T16:04:24Z

avogadro/qtplugins/label/label.cpp

+        if (i == 0) {
+          atom->addItem(QObject::tr("None"), QVariant(LabelOptions::None));
+        } else {
+          char val = 0x00;


Just use a string list. I know this works in English, but it won't work for translation in many languages.

QStringList options;
options << tr("None") << tr("Index") << tr("Element Symbol") << tr("Symbol + Index"); // etc

ghutchis · 2021-08-23T16:05:21Z

avogadro/qtplugins/label/label.cpp

+      text += (text == "" ? "" : " / ") + atom.label();
+    }
+    if (interface.atomOptions & LayerLabel::LabelOptions::Index) {
+      text += (text == "" ? "" : " / ") + std::to_string(atom.index());


atom.index() + 1 // most users expect Atom 1, Atom 2, etc. rather than starting at zero...

Yes, but the "Select by Atom Index" is indexed at 0. I can change both elements to indexed at 1 if you prefer.

Yes, I think most users expect atom numbers to start at 1.

Signed-off-by: Marc Prat Masó <marc.prat.maso@estudiantat.upc.edu>

serk12 · 2021-08-25T07:25:31Z

Though adding all the combinations looks noisy, if a user needs any of them it will be complex (and probably ugly). I can try a custom sort so it's more appealing or just remove them.

github-actions · 2021-08-25T08:59:41Z

Here are the build results
macOS.dmg
Ubuntu-2004.tar.gz
Win64.exe
Artifacts will only be retained for 90 days.

ghutchis · 2021-08-27T19:45:46Z

I can try a custom sort so it's more appealing or just remove them.

Yes, I'd just remove them from the list. If users ask later, it's easy enough to add a few. But I'd rather go for "the most common" list now. It's the 80/20 or 90/10 rule .. maybe 20% of the options are used 80% of the time.

It's always a challenge with our UI because it's used both by experts doing research as well as students doing molecular visualization. I figure most of the users are on the student end (so keep things simple).

Signed-off-by: Marc Prat Masó <marc.prat.maso@estudiantat.upc.edu>

github-actions · 2021-08-31T12:29:43Z

Here are the build results
macOS.dmg
Ubuntu-2004.tar.gz
Win64.exe
Artifacts will only be retained for 90 days.

ghutchis · 2021-08-31T18:55:18Z

I think it's a great start. I might tweak the icon a bit (e.g., a letter in front of an atom).

There's a lot of interest in scripts adding labels to atoms, so this is great. Need to add the atom labels to the property table too.

label editor and fix transparent label

9299907

Signed-off-by: Marc Prat Masó <marc.prat.maso@estudiantat.upc.edu>

ghutchis reviewed Aug 19, 2021

View reviewed changes

small fixes

259bd97

Signed-off-by: Marc Prat Masó <marc.prat.maso@estudiantat.upc.edu>

qcombobox, and procedural generator

0d0cf2d

Signed-off-by: Marc Prat Masó <marc.prat.maso@estudiantat.upc.edu>

ghutchis reviewed Aug 23, 2021

View reviewed changes

case typos and QStringList

b44356e

Signed-off-by: Marc Prat Masó <marc.prat.maso@estudiantat.upc.edu>

remove extra label options

93235e1

Signed-off-by: Marc Prat Masó <marc.prat.maso@estudiantat.upc.edu>

ghutchis added enhancement feature changes / API changes rendering labels Aug 31, 2021

ghutchis changed the title ~~label editor and fix transparent label~~ Add label editor to create custom labels Aug 31, 2021

ghutchis merged commit 6de14ab into OpenChemistry:master Aug 31, 2021

serk12 deleted the label_editor branch September 9, 2021 09:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add label editor to create custom labels #740

Add label editor to create custom labels #740

serk12 commented Aug 19, 2021

ghutchis Aug 19, 2021

serk12 Aug 20, 2021

ghutchis Aug 22, 2021

ghutchis Aug 19, 2021

serk12 Aug 20, 2021

ghutchis Aug 19, 2021

ghutchis commented Aug 19, 2021

ghutchis commented Aug 19, 2021

github-actions bot commented Aug 19, 2021

avo-bot commented Aug 19, 2021

serk12 commented Aug 20, 2021

ghutchis commented Aug 20, 2021

ghutchis commented Aug 20, 2021

github-actions bot commented Aug 21, 2021

ghutchis commented Aug 22, 2021

github-actions bot commented Aug 23, 2021

ghutchis Aug 23, 2021

ghutchis Aug 23, 2021

ghutchis commented Aug 23, 2021

ghutchis Aug 23, 2021

ghutchis Aug 23, 2021

serk12 Aug 25, 2021

ghutchis Aug 29, 2021

serk12 commented Aug 25, 2021

github-actions bot commented Aug 25, 2021

ghutchis commented Aug 27, 2021

github-actions bot commented Aug 31, 2021

ghutchis commented Aug 31, 2021

Add label editor to create custom labels #740

Add label editor to create custom labels #740

Conversation

serk12 commented Aug 19, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ghutchis commented Aug 19, 2021

ghutchis commented Aug 19, 2021

github-actions bot commented Aug 19, 2021

avo-bot commented Aug 19, 2021

serk12 commented Aug 20, 2021

ghutchis commented Aug 20, 2021

ghutchis commented Aug 20, 2021

github-actions bot commented Aug 21, 2021

ghutchis commented Aug 22, 2021

github-actions bot commented Aug 23, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ghutchis commented Aug 23, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

serk12 commented Aug 25, 2021

github-actions bot commented Aug 25, 2021

ghutchis commented Aug 27, 2021

github-actions bot commented Aug 31, 2021

ghutchis commented Aug 31, 2021