Skip to content
This repository has been archived by the owner on Jul 1, 2022. It is now read-only.

Google Translate block #1017

Merged
merged 16 commits into from May 9, 2018
Merged

Conversation

rotator
Copy link
Contributor

@rotator rotator commented Apr 26, 2018

Steps for review

After installation or update OpenY profile

@podarok please, review

Copy link
Contributor

@podarok podarok left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs some love

$('nav .navbar-nav li.language > a').on('click', function (e, context) {
e.preventDefault();
langSelect.css('left', '0px');
langSelect.css('top', '150px');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you operate by classes and avoid of adding styles int the code?
MVC pattern is really important here

type: module
description: OpenY module for Google Translate integration.
core: 8.x
package: OpenY
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add version, please

@@ -0,0 +1,8 @@
translate:
version: 1.x
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use Semantic Versioning. Should be 8.0.1, please

public function build() {
$block = [
'#theme' => 'openy_gtranslate',
'#cache' => ['max-age' => 0],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why remove caching? Could this block be cached?
It is on every page, I'm not a big fan of adding not cached blocks for all pages

<div class="google-translate" id="google_translate_element"></div>
<script type="text/javascript">
function googleTranslateElementInit() {
new google.translate.TranslateElement({pageLanguage: 'en', layout: google.translate.TranslateElement.InlineLayout.SIMPLE}, 'google_translate_element');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Language code should be passed from Drupal site settings, not hardcoded

@ymcatwincities ymcatwincities deleted a comment from gundevel May 8, 2018
@ymcatwincities ymcatwincities deleted a comment from gundevel May 8, 2018
@podarok
Copy link
Contributor

podarok commented May 8, 2018

image
Looks like we should check our styles before merging too

@ymcatwincities ymcatwincities deleted a comment from gundevel May 8, 2018
@ymcatwincities ymcatwincities deleted a comment from gundevel May 8, 2018
@ymcatwincities ymcatwincities deleted a comment from gundevel May 8, 2018
@gundevel
Copy link
Collaborator

gundevel commented May 8, 2018

Build comment file:

Environment Link
Fresh OpenY installation http://openy.cibox.tools/build338
Upgraded(upgrade path) installation http://upgrade.openy-dev.ffwua.com/build338
Installation process http://install.openy-dev.ffwua.com/build338/install.php

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
http://openy.cibox.tools:8080/job/PR_BUILDER_COMPOSER/338/

@gundevel
Copy link
Collaborator

gundevel commented May 8, 2018

Acessibility Sniffer: front page checking WCAG2AA http://ci.openymca.org/build338/frontWCAG2AAhtmlcs.txt
Acessibility Sniffer: join page checking WCAG2AA http://ci.openymca.org/build338/joinWCAG2AAhtmlcs.txt
Acessibility Sniffer: locations page checking WCAG2AA http://ci.openymca.org/build338/locationsWCAG2AAhtmlcs.txt
Acessibility Sniffer: schedules page checking WCAG2AA http://ci.openymca.org/build338/schedulesWCAG2AAhtmlcs.txt
Acessibility Sniffer: blog page checking WCAG2AA http://ci.openymca.org/build338/blogWCAG2AAhtmlcs.txt

@gundevel
Copy link
Collaborator

gundevel commented May 8, 2018

Build comment file:

Environment Link
Fresh OpenY installation http://openy.cibox.tools/build339
Upgraded(upgrade path) installation http://upgrade.openy-dev.ffwua.com/build339
Installation process http://install.openy-dev.ffwua.com/build339/install.php

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
http://openy.cibox.tools:8080/job/PR_BUILDER_COMPOSER/339/

@gundevel
Copy link
Collaborator

gundevel commented May 8, 2018

Acessibility Sniffer: front page checking WCAG2AA http://ci.openymca.org/build339/frontWCAG2AAhtmlcs.txt
Acessibility Sniffer: join page checking WCAG2AA http://ci.openymca.org/build339/joinWCAG2AAhtmlcs.txt
Acessibility Sniffer: locations page checking WCAG2AA http://ci.openymca.org/build339/locationsWCAG2AAhtmlcs.txt
Acessibility Sniffer: schedules page checking WCAG2AA http://ci.openymca.org/build339/schedulesWCAG2AAhtmlcs.txt
Acessibility Sniffer: blog page checking WCAG2AA http://ci.openymca.org/build339/blogWCAG2AAhtmlcs.txt

@ymcatwincities ymcatwincities deleted a comment from gundevel May 9, 2018
@podarok podarok merged commit 43b0324 into ymcatwincities:8.x-1.x May 9, 2018
@ymcatwincities ymcatwincities deleted a comment from gundevel Jul 16, 2018
@ymcatwincities ymcatwincities deleted a comment from gundevel Jul 16, 2018
@ymcatwincities ymcatwincities deleted a comment from gundevel Jul 16, 2018
@ymcatwincities ymcatwincities deleted a comment from gundevel Jul 16, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants