greenpeace / planet4

Built on top of Wordpress tech, Greenpeace Planet 4 powers digital platforms to engage with millions and win campaigns around the world.
https://planet4.greenpeace.org
Creative Commons Attribution Share Alike 4.0 International
66 stars 27 forks source link

PLANET-5615 Use button tags for buttons #125

Closed planet-4 closed 3 years ago

planet-4 commented 3 years ago

Currently a lot of our buttons use anchor tags, that's not good in terms of accessibility, and also causes issues when trying to disable them. We should use

aeisenberg commented 3 years ago

I'll take a look at this one.

aeisenberg commented 3 years ago

There are more a tags, but I am having trouble converting the remainder. Either, I believe they should remain as <a> or I don't see them being used in the repos at all, so I do not want to touch them since I can't see what they look like.

The following are anchors I don't know how to use:

planet4-docker-compose/persistence/app/public/wp-content/themes/planet4-master-theme/assets/src/js/country_select.js:
  28        '<div id="country-list" class="country-list">' +
  29:       '<a class="international" href=""></a>' +
  30        '<ul class="countries_list" aria-label="' + __('Worldwide site list', 'planet4-master-theme') + '" role="list"></ul>' +

  55                '<li>' +
  56:                 '<a ' +
  57                    'href="' + lang.url + '" ' +

planet4-docker-compose/persistence/app/public/wp-content/themes/planet4-master-theme/assets/src/js/Components/archivePicker/SingleSidebar.js:
  55            __('URL', 'planet4-master-theme-backend'),
  56:           <a href={original.url}>{original.url} </a>
  57          )}

planet4-docker-compose/persistence/app/public/wp-content/plugins/planet4-plugin-gutenberg-blocks/assets/src/blocks/Splittwocolumns/SplittwocolumnsFrontend.js:
  93            {button_text && button_link &&
  94:             <a className="btn btn-primary btn-block split-two-column-item-button"
  95                  href={button_link}

The following are anchors I think should remain anchors:

planet4-docker-compose/persistence/app/public/wp-content/plugins/planet4-plugin-gutenberg-blocks/assets/src/blocks/Articles/ArticlePreview.js:
  47              :
  48:             <a href={authorLink}>{authorName}</a>
  49            }

planet4-docker-compose/persistence/app/public/wp-content/plugins/planet4-plugin-gutenberg-blocks/assets/src/blocks/Splittwocolumns/SplittwocolumnsFrontend.js:
  46              <h2 className="split-two-column-item-title">
  47:               <a href={issue_link_path} {...analytics('Category Title')}
  48                  dangerouslySetInnerHTML={{__html: title}}

  60            {issue_link_text && issue_link_path &&
  61:             <a className="split-two-column-item-link"
  62                 href={issue_link_path}

  81            {tag_name &&
  82:             <a className="split-two-column-item-tag"
  83                 href={tag_link}

  93            {button_text && button_link &&
  94:             <a className="btn btn-primary btn-block split-two-column-item-button"
  95                  href={button_link}

planet4-docker-compose/persistence/app/public/wp-content/plugins/planet4-plugin-gutenberg-blocks/assets/src/blocks/Splittwocolumns/SplittwocolumnsInPlaceEdit.js:
  101            {tag_name &&
  102:             <a className="split-two-column-item-tag" href={tag_link}>
  103                #{tag_name}

planet4-docker-compose/persistence/app/public/wp-content/plugins/planet4-plugin-gutenberg-blocks/assets/src/blocks/Timeline/URLDescriptionHelp.js:
  10    )
  11:   + '<br><a href="https://timeline.knightlab.com/#make" target="_blank" rel="noopener noreferrer">'
  12    + __(

In the first block, could someone please describe how I can add these components to a page, and for the second block. please let me know if there are any anchors you believe really should be converted.

comzeradd commented 3 years ago

Pinging @mleray and @pablocubico who have spent more time into this.

mleray commented 3 years ago

@aeisenberg Thanks for looking into this, I'll try to answer your questions! We should have more made it clear in the ticket, but the only a tags that need to be converted to button tags are the ones that are used as buttons. You can recognise them because they have the .btn class, usually together with either btn-primary, btn-secondary and btn-donate.

So in your previous comment, indeed most of them do not need the conversion except for this one:

planet4-docker-compose/persistence/app/public/wp-content/plugins/planet4-plugin-gutenberg-blocks/assets/src/blocks/Splittwocolumns/SplittwocolumnsFrontend.js:
  93            {button_text && button_link &&
  94:             <a className="btn btn-primary btn-block split-two-column-item-button"
  95                  href={button_link}

I hope that helps? 🙂

mleray commented 3 years ago

Update: we discussed this issue a bit more within the P4 team and actually it's not the best to convert all of our anchor tags into buttons… That would mean that we need to implement in JS the href, opening in new tabs etc and I didn't really think it through when creating the ticket. From a quick accessibility search, the problem is more that we make links look like buttons, which is something our design team would need to work on before we can make changes. The only component that needed actual changes was the GalleryCarousel and it's been done (thank you @aeisenberg 🙏 ) so we can now close this issue.