Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Social Media Channels - More ways to connect #2391

Merged
merged 1 commit into from
Jul 16, 2024

Conversation

SebastianBurke
Copy link
Contributor

@SebastianBurke SebastianBurke commented Jul 9, 2024

image

Copy link
Contributor

@Garneauma Garneauma left a comment

Choose a reason for hiding this comment

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

Please add the link inside the include file instead of the working example page and add a setting in the include file like "if moreWays == true". You can add that setting to the include command in the "more ways to connect" working example.

The "gruntfile.coffee" should not be in this PR. Please revert this change.

You should always git fetch --all and git reset --hard upstream/master whenever you start working on a new PR.

Let me know if you have any question. I'll be happy to help.

Copy link
Contributor

@Garneauma Garneauma left a comment

Choose a reason for hiding this comment

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

Partial review.

Please alse update the French working examples. Thanks!

components/gc-follow-us/includes/gc-follow-us.html Outdated Show resolved Hide resolved
components/gc-follow-us/includes/gc-follow-us.html Outdated Show resolved Hide resolved
components/gc-follow-us/index.json-ld Outdated Show resolved Hide resolved
@SebastianBurke SebastianBurke changed the title Looking for feedback - Social media update Social Media Channels - More ways to connect Jul 11, 2024
Copy link
Contributor

@Garneauma Garneauma left a comment

Choose a reason for hiding this comment

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

  • Please remove all occurrences of defined parameters defined to false in the include commands.
  • Add history item to log this change at line 74 of index.json-ld.
  • The inline version with more ways to connect needs to be adjusted. You will need to add a class to this <li> like class="more-ways" and some custom styling in _base.scss. so that it wraps under and does not have the styles of the other elements. I suggest editing line 128 to li:not(.more-ways).

components/gc-follow-us/gc-follow-us-en.html Outdated Show resolved Hide resolved
components/gc-follow-us/gc-follow-us-en.html Outdated Show resolved Hide resolved
components/gc-follow-us/gc-follow-us-en.html Outdated Show resolved Hide resolved
components/gc-follow-us/gc-follow-us-en.html Outdated Show resolved Hide resolved
components/gc-follow-us/gc-follow-us-fr.html Outdated Show resolved Hide resolved
components/gc-follow-us/gc-follow-us-fr.html Outdated Show resolved Hide resolved
components/gc-follow-us/gc-follow-us-fr.html Outdated Show resolved Hide resolved
components/gc-follow-us/gc-follow-us-fr.html Outdated Show resolved Hide resolved
Copy link
Contributor

@Garneauma Garneauma left a comment

Choose a reason for hiding this comment

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

One small change. Should be good after that.

components/gc-follow-us/_base.scss Outdated Show resolved Hide resolved
@Garneauma
Copy link
Contributor

Pre-approved upon successful review. Waiting for formal DTO approval.

@Garneauma
Copy link
Contributor

@SebastianBurke, DTO just came back and would like for the link to be underlined. Please more the ".more ways" definition to line 105 right under "&:last-child". Please also make sure it reads "&.more-ways". Also, add "text-decoration: underline" to the "a" inside ".more-ways". Thank you.

components/gc-follow-us/index.json-ld Outdated Show resolved Hide resolved
components/gc-follow-us/index.json-ld Outdated Show resolved Hide resolved
@Garneauma Garneauma merged commit 10fb183 into wet-boew:master Jul 16, 2024
1 check passed
@Garneauma Garneauma added this to the v15.4.0 milestone Jul 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants