Schedule editor constraint icons not shown

[ietf-svn-bot]

type_defect | by jennifer@painless-security.com

---

The small icons for session constraints in the schedule editor are no longer shown. I believe this is because they are still using the font-awesome classes. These should be updated to equivalent or similar bootstrap icons.

This affects the bootstrap5 revamp

---

Issue migrated from trac:3557 at 2022-03-04 09:29:12 +0000

[ietf-svn-bot]

@rjsparks@nostrum.com changed status from new to accepted

[ietf-svn-bot]

@rjsparks@nostrum.com commented

---

Is there an implicit (or explicit) decision to abandon font-awesome in this branch?

[ietf-svn-bot]

@jennifer@painless-security.com commented

---

It appears that old fa items have been replaced at least fairly systematically with Bootstrap's bi classes. I'm not previously familiar with these, but they look to be at least superficially similar to font-awesome.

[ietf-svn-bot]

@jennifer@painless-security.com commented

---

It appears to have been changed in 1d55937

[ietf-svn-bot]

@lars@eggert.org commented

---

I switched us from FontAwesome to Bootstrap Icons, because recent versions of the former are no longer free, and the switch eliminated an additional external asset.

[larseggert]

@jennifer-richards could you point me at the code where this use of font-awesome happens? I just looked and couldn't find any remaining ones (modulo the one I fixed in #3623, but which is not in the schedule editor.)

[jennifer-richards]

Ah, it's the ConstraintName.editor_label values in the database. It looks like it was fixed in names.json but a migration is needed to change those on the production system.

>>> pp(list(ConstraintName.objects.filter(editor_label__contains='fa-').values_list('slug', 'editor_label')))
[('bethere', '<i class="fa fa-user-o"></i>{count}'),
 ('timerange', '<i class="fa fa-calendar-o"></i>'),
 ('wg_adjacent', '<i class="fa fa-step-forward"></i>'),
 ('chair_conflict', '<i class="fa fa-gavel"></i>'),
 ('tech_overlap', '<i class="fa fa-rocket"></i>'),
 ('key_participant', '<i class="fa fa-key"></i>')]

[larseggert]

Ah. I can't help with that :-)

[jennifer-richards]

No problem, I'll get it.

[rjsparks]

We need to review all the places where we allow raw html into the database.
For this particular case - when building the migration, lets talk about whether refactoring to move the knowledge of the html off to code is the right thing to do.

[jennifer-richards]

For reference all of the ConstraintName labels are:

[('conflict', '<span class="encircled">1</span>'),
 ('conflic2', '<span class="encircled">2</span>'),
 ('conflic3', '<span class="encircled">3</span>'),
 ('bethere', '<i class="fa fa-user-o"></i>{count}'),
 ('timerange', '<i class="fa fa-calendar-o"></i>'),
 ('time_relation', '&Delta;'),
 ('wg_adjacent', '<i class="fa fa-step-forward"></i>'),
 ('chair_conflict', '<i class="fa fa-gavel"></i>'),
 ('tech_overlap', '<i class="fa fa-rocket"></i>'),
 ('key_participant', '<i class="fa fa-key"></i>')]

IMO a sensible thing to do would be to replace the raw HTML in the DB with a string that identifies the icon to show. Generally that would just be translated directly into the bootstrap icon <i> element, but special cases like 'encircled-1' could be intercepted and rendered as <span>.

In the future when something else replaces bootstrap-icon, we just translate the necessary codes without needing a migration.

This doesn't solve the issue entirely, though, because we'd still have to keep track of which bootstrap icons are actually in use so we know to translate them. We could avoid that by only providing a limited set of icons and not passing unknown values through to the bootstrap-icon library, but in that case I think we might as well hard code the editor_labels.

Is there an actual need to redefine these things on the fly?

[rjsparks]

It's the way it is because we were exploring unknown territory and were looking to make it easy to experiment with changes on live data without software updates. That need has since passed, and I agree with the direction you propose.

[jennifer-richards]

After some offline discussion with Robert, I'm planning to eliminate the editor_label field from the ConstraintName field. Instead, I'll add a template tag (or perhaps filter, tbd) that generates the appropriate template contents. So, e.g., instances of {{ constraint.name.editor_label }} will become {% editor_label_for constraint %} or {{ constraint|as_editor_label }}