Rewrite update components inline with our CSS standards

Took a quick look to this one.

We have 4 templates affecting css mentioned in issue description: update-last-check.html.twig, update-version.html.twig, update-project-status.html.twig, update-report.status.html.twig

Most of them are outputting classes that are not consistent with parts we have already renewed.

Starting to look through these.
update-version.html.twig
Current:

<table class="{{ attributes.class }} version"{{ attributes|without('class') }}>
  <tr>
    <td class="version-title">{{ title }}</td>
    <td class="version-details">
      <a href="{{ version.release_link }}">{{ version.version }}</a>
      <span class="version-date">({{ version.date|date('Y-M-d') }})</span>
    </td>
    <td class="version-links">
      <ul class="links">
        <li class="download">
          <a href="{{ version.download_link }}">{{ 'Download'|t }}</a>
        </li>
        <li class="release-notes">
          <a href="{{ version.release_link }}">{{ 'Release notes'|t }}</a>
        </li>
      </ul>
    </td>
  </tr>
</table>

Proposed:

<table class="{{ attributes.class }} update-version"{{ attributes|without('class') }}>
  <tr>
    <td class="update__version-title">{{ title }}</td>
    <td class="update__version-details">
      <a href="{{ version.release_link }}">{{ version.version }}</a>
      <span class="update__version-date">({{ version.date|date('Y-M-d') }})</span>
    </td>
    <td class="update__version-links">
      <ul class="foo">
        <li class="update__version-links__download">
          <a href="{{ version.download_link }}">{{ 'Download'|t }}</a>
        </li>
        <li class="update__version-links__release-notes">
          <a href="{{ version.release_link }}">{{ 'Release notes'|t }}</a>
        </li>
      </ul>
    </td>
  </tr>
</table>

But it feels like update__version-links__download and update__version-links__release-notes are a bit long classes.

Update.report.html.twig
Current:

{{ last_checked }}

{% for project_type in project_types %}
  <h3>{{ project_type.label }}</h3>
  {{ project_type.table }}
{% else %}
  <p>{{ no_updates_message }}</p>
{% endfor %}

Proposed:

{{ last_checked }}

{% for project_type in project_types %}
  <h3 class="update__report-title">{{ project_type.label }}</h3>
  {{ project_type.table }}
{% else %}
  <p class="update__report-no-messages">{{ no_updates_message }}</p>
{% endfor %}

update.last.check.html.twig

Current:

<div class="update checked">
  {% if last %}
    {{ 'Last checked: @time ago'|t({'@time': time}) }}
  {% else %}
    {{ 'Last checked: never'|t }}
  {% endif %}
  <span class="check-manually">({{ link }})</span>
</div>

Proposed:

<div class="update__checked">
  {% if last %}
    {{ 'Last checked: @time ago'|t({'@time': time}) }}
  {% else %}
    {{ 'Last checked: never'|t }}
  {% endif %}
  <span class="update__check-manually">({{ link }})</span>
</div>

Last of the four is so large that I will check it on another day.

It looks like a lot of classes of he css-file are coming from places such as core/modules/update/src/Form/UpdateManagerUpdate.php. It looks like we will need a lot of work to get this inline with our standards.

Yeah.

I also want to mention that testing this issue is actually quite hard. Installed new Drupal 8 and few contribs like restui, date and omega theme.

The only line from update.admin.css that got triggered on Bartik, Stark, Classy and Seven was line 163:

th.update-project-name {
  width: 50%;
}

But I'm pretty sure some css gets triggered if we have correct kind of updates available.

Ok, managed to figure out what this is actually doing. It seems that I have a lot to do on trying to explain things (pointing to IRC discussion with @lewisnyman).

Difficult thing on testing this issue and what our CSS and templates are currently doing is that we are working for example on admin/modules/update -page and it's output is related on what kind of modules OR core we have in need of update/upgrade.

Options are:
1. Major update
2. Security patch needed
3. Module not supported anymore
4. Update information unknown
5. We have a valid version

There is different CSS for every scenario

As we're working on 8.0.x branch, we don't have any (or do we?) options on modules that are in need of security patch nor which are not supported anymore. Also upgrade from 8.0.x to 8.0.0-beta-6 is not considered as major.

So, we need to actually hack core a bit to see if templates and CSS is working properly (or this was the easiest way for me to do it). To get "major update needed" text and it's styles visible:

You need to change line #139 from core/modules/update/src/Form/UpdateManagerUpdate.php from

 if ($recommended_release['version_major'] != $project['existing_major']) {

to

 if (true or $recommended_release['version_major'] != $project['existing_major']) {

(^^or that is the easiest way I managed to do it)

To test the CSS on other scenarios (=security patch needed, not supported anymore or update information unknown -part) you need to make similar kind of hack to same file to switch sentence starting from line #163.

But I can also assure that CSS related to this is currently working and there's no extra lines, so we just need to check that CSS is written with coding standards and document it, as the few lines we're using on /admin/modules/update are splitted all over the CSS-file.

Other things

update.admin.css is also being triggered on /admin/reports/updates, /admin/reports/updates/settings, /admin/update/ready and /admin/theme/install

Same issues on testing the current (or modified) CSS are also here than what we have on admin/modules/update. We should have modules OR core in need for updates for different reasons to see that everything is being triggered as it should. With similar kind of hacks I checked the current CSS and it looks like it's pretty clear and there isn't at least much stuff that is not being triggered.

Clean up

When our CSS-file is full of things like

.update tr.warning .version {
  background: #ffe;
}

.current-version,
.new-version {
  direction: ltr; /* Note: version numbers should always be LTR. */
}

.update tr.unknown {
  background: #ddd;
}

table.update,
.update table.version {
  width: 100%;
  margin-top: .5em;
  border: none;
}

It's highly unclear if that is used in /admin/modules/update or /admin/reports/update or somewhere else (or at all). I do know that we don't want to increase the amount of CSS or CSS-files in core as, so the CSS file should be documented well.

Twig files are not enough as some of the classes are coming from core/modules/update/src/Form/UpdateManagerUpdate.php and I think that some are coming from core/modules/update/update.compare.inc. I think that some of the table related things are still coming from somewhere else, but didn't have enough time to figure it out, yet.