Meta issue: #1980004: [meta] Creating Dream Markup
Issue based on: #1939100: Convert theme_progress_bar() to Twig

Questions:

Why we are using classes lide .bar .message, when they tell nothing about our module?
Why use div for text wrapping?

Markup proposal:

<div id="progress" class="progress">
  <div class="progress-bar">
    <div class="progress-filled" style="width: {{ percent }}%;"></div>
  </div>
  <p class="progress-percentage">{{ percent }}%</p>
  <p class="progress-message">{{ message }}</p>
</div>
CommentFileSizeAuthor
#6 progress-bar.png328.3 KBrteijeiro
#5 1982220-progress-bar-dreammarkup.patch4.58 KBKarmen
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

oresh’s picture

oresh CreditAttribution: oresh commented
Project: » Drupal core
Version: » 8.x-dev
Component: Twig templates conversion (front-end branch) » markup
Category: feature » task

moving issue to core.

LewisNyman’s picture

LewisNyman
he/him
CreditAttribution: LewisNyman commented

RELATED: #1989480: Progress Bar style update

ry5n’s picture

ry5n CreditAttribution: ry5n commented

Markup from #1989480: Progress Bar style update, with notes and room for improvement:

<div class="progress">
  <!-- .label is the same style used for form labels -->
  <label class="label">Installing Node Module</label>
  <div class="progress__track">
    <div class="progress__bar" style="width: 63%"></div>
  </div>
  <div class="progress__description">
    <!-- .l-pull and l.push are utility classes for floating -->
    <div class="l-pull">Installed 15 of 24 modules</div>
    <strong class="l-push">63%</strong>
  </div>
  <!-- this could be refactored to a general '.icon-close' -->
  <a class="progress__close" href="#" title="cancel">cancel</a>
</div>

BTW, this is how it looks with Seven theme styles and a small variant: http://drupalcode.org/sandbox/ry5n/1932040.git/blob_plain/HEAD:/patterns...

star-szr’s picture

star-szr
he/him
Primary language English
CreditAttribution: star-szr commented
Issue tags: +dreammarkup

Tagging.

Karmen’s picture

Karmen CreditAttribution: Karmen commented
Issue summary: View changes
Status: Active » Needs review
FileSize
1982220-progress-bar-dreammarkup.patch4.58 KB

I've changed the markup of the template according with the suggested markup. I've left the id because it was needed in js.

Be nice! It's my first contribution!

rteijeiro’s picture

rteijeiro CreditAttribution: rteijeiro commented
Status: Needs review » Reviewed & tested by the community
FileSize
progress-bar.png328.3 KB

It's a RTBC for me. Progress bar looks shiny and markup seems to be right.

Progress Bar

Congrats @Karmen for your first patch!!

LewisNyman’s picture

LewisNyman
he/him
CreditAttribution: LewisNyman commented
Status: Reviewed & tested by the community » Needs review

It looks like the markup in the first post is out of date? The suggestions in #103 looks more inline with our standards to me?

rteijeiro’s picture

rteijeiro CreditAttribution: rteijeiro commented

@LewisNyman what are the suggestions? Do you mean in this issue #1989480: Progress Bar style update ?

LewisNyman’s picture

LewisNyman
he/him
CreditAttribution: LewisNyman commented

Yeah it looks like the markup that was implemented there is pretty close to what we want

nod_’s picture

nod_
Primary language French
Location Lille
CreditAttribution: nod_ commented
Issue tags: +JavaScript

Don't forget the tag :)

Dragan Eror’s picture

Dragan Eror CreditAttribution: Dragan Eror commented

Here is some additional style improvement for progress bar... #2254785: Progress bar 0% or over 100% visual BUG

LewisNyman’s picture

LewisNyman
he/him
CreditAttribution: LewisNyman commented
Status: Needs review » Closed (works as designed)
Issue tags: +frontend

I'm going to close this issue as it predates #1989480: Progress Bar style update which changed the default mark up. If someone still feel like the mark up needs improving then please reopen this issue and update the issue summary.