Feature/footer social media icons - option 1 #60

aimeelramirez · 2022-01-03T12:48:14Z

Related Issue

Resolves #51

Summary of Changes

created a new file "social-media.ts".
applied styles on "social-share.css".
container app-socials-media is shown on "footer.ts". It can be shown on "header.ts" but it looks best on the footer if approved.

…called social-media.ts - created a new file social-media.ts - applied styles on social-share.css - container is shown on footer. It can be shown on header but it looks best on the footer if approved.

netlify · 2022-01-03T12:48:23Z

✔️ Deploy Preview for practical-fermat-fa2c48 ready!

🔨 Explore the source changes: dcea933

🔍 Inspect the deploy log: https://app.netlify.com/sites/practical-fermat-fa2c48/deploys/61da7b2fb78eca0007867950

😎 Browse the preview: https://deploy-preview-60--practical-fermat-fa2c48.netlify.app/

thescientist13 · 2022-01-04T14:27:39Z

Awesome, will definitely be reviewing this in a little bit.

thescientist13

This looks nice, and thanks for adding the test!

I'm thinking, given this is a one off use case and the links / content are all static, it might make more sense to avoid extra JavaScript by making an entire WC for it, and just having the HTML hardcoded right in the footer instead? (my short term goal in general is to migrate this project to be more static / hybrid and less SPA, so this would also aid in the effort as well)

I'm reaching out now to get the Instagram link for you. 👍

thescientist13 · 2022-01-04T16:14:34Z

src/components/footer/footer.ts

+            <p class='copyright-text'>&copy; ${STARTING_YEAR} - ${currentYear} Analog Studios</p>
+          </div>
+          <div class='col-xs-12'>
+            <app-socials-media></app-socials-media>


I'm thinking maybe it would be better to put these links above the copyright? Also maybe making them a little bigger?

thescientist13 · 2022-01-04T16:16:05Z

src/components/footer/footer.spec.js

@@ -23,6 +23,12 @@ describe('Footer Component', () => {
      expect(footer.shadowRoot.querySelectorAll('footer').length).equal(1);
    });

+    it('should have two div containers', () => {
+      const elements = footer.shadowRoot.querySelectorAll('div.row > div');


maybe we could test for something a little more concrete? Like N number of <a> tags to capture each link added?

thescientist13 · 2022-01-04T18:15:10Z

@aimeelramirez
Here is the instagram link
https://www.instagram.com/dave.flamand/

aimeelramirez · 2022-01-05T06:41:01Z

This looks nice, and thanks for adding the test!

I'm thinking, given this is a one off use case and the links / content are all static, it might make more sense to avoid extra JavaScript by making an entire WC for it, and just having the HTML hardcoded right in the footer instead? (my short term goal in general is to migrate this project to be more static / hybrid and less SPA, so this would also aid in the effort as well)

I'm reaching out now to get the Instagram link for you. 👍

Thanks, Owen(@thescientist13)! I was trying to apply the same coding conventions as you had done previously 😓. I'm happy to hardcode it to meet the requirements on WC. 😁
Quick questions:

Would you consider to upgrade the version 4 to 5 on Font Awesome/package manager installs?
If to statically (not dynamically) host the site, we can always save it in the source tree and reference it. Moreover, we can bundle and minify the asset directory to optimize the loading time, if interested:
fontawesome-free-5.15.4-web.zip.

Let me know what you think! 😅

Sincerely,
aimee

thescientist13 · 2022-01-05T15:47:07Z

Thanks for the good questions / feedback. 👍

I was trying to apply the same coding conventions as you had done previously 😓. I'm happy to hardcode it to meet the requirements on WC.

All good. I think the key distinction here for me anyway is that these are just links to websites, and is not a sharing mechanism per se, so no JavaScript or interactivity is really needed when all we're really talking about is an <a> tag at the end of the day.

The existing social share component is different in that it has to be dynamic per page / instance. In other words, sharing URLs like /artists/1 vs /artists/2 is something that can only be done at runtime when the user clicks the share button, so that it can shared through the social media platform chosen by the user on the fly.

These social links are always going to stay the same on every page since it's well, just a link. :)

In addition, it's just a lot less code.

<a href="https://youtube.com/....."><i>.....</i></a>
<a href="https://facebook.com/....."><i>.....</i></a>
<a href="https://instagram.com/....."><i>.....</i></a>

In general if it can be done without JavaScript, ideally I opt for that first. I'm sure there will be plenty of opportunities to write some JavaScript / WCs, but not sure it's the best solution for this particular task. 🤓

Would you consider to upgrade the version 4 to 5 on Font Awesome/package manager installs?

Sure, If you want to give it a shot in another PR I'm fine with that.

It wouldn't be work I would sponsor at the moment since I'm still thinking if I want to redesign this site or not and my main goal right now is to convert more of the dynamic content to static and go in a more hybrid direction, so that is top of mind at the moment for me. You can see #37 for more info on that discussion.

Moreover, we can bundle and minify the asset directory to optimize the loading time, if interested:

It is already is bundled with the site and under package management, as well as minified, unless I am misunderstanding your suggestion? That said, due to Shadow DOM, it is being overly bundled / duplicated but I have that solution almost implemented in #56 .

edit: Thinking on it some more, I would be open with at least evaluating the upgrade to Bootstrap, at which point doing the same for FA might also be valuable. Made a discussion to continue that convo as a sponsorship candidate - #61

Related to #60  1. [x] created and validated W3C development tools upon creating tags "footer.ts." 2. [x] applied styles on "footer.css." 3. [x] applied test spec to select anchor tags to be found.

* Resolves #51 Related to #60  1. [x] created and validated W3C development tools upon creating tags "footer.ts." 2. [x] applied styles on "footer.css." 3. [x] applied test spec to select anchor tags to be found. * headline tag for w3c tests * final fixes until review upon acceptance * final fixes until review upon acceptance on ' * html to have " in reviewing files * formatting the tasks completed upon switching links above copyright, test spec on nth of type, and css style to space evenly within host * add spacing between icons * open social links in new window Co-authored-by: Owen Buckley <owenbuckley13@gmail.com>

applying changes to footer and created a new file under social share …

6834e5f

…called social-media.ts - created a new file social-media.ts - applied styles on social-share.css - container is shown on footer. It can be shown on header but it looks best on the footer if approved.

aimeelramirez added the chore label Jan 3, 2022

aimeelramirez added 3 commits January 3, 2022 07:53

removal web-share if not wanting to utilized

6f24e76

minor fixes and checked with yarn lint

852209e

Adding tests on the footer to validate on adding socials

1731b92

thescientist13 added feature New feature or request and removed chore labels Jan 4, 2022

thescientist13 self-requested a review January 4, 2022 16:13

thescientist13 added this to IN REVIEW in Jamstack Migration - Phase 1 Jan 4, 2022

thescientist13 reviewed Jan 4, 2022

View reviewed changes

thescientist13 removed this from IN REVIEW in Jamstack Migration - Phase 1 Jan 4, 2022

thescientist13 assigned aimeelramirez Jan 8, 2022

updating the option 1 on hardcoded section on footer element

65815ab

yarn lint on ' and on testing

dcc35e4

aimeelramirez mentioned this pull request Jan 9, 2022

Feature/footer social media icons - option 2 #64

Merged

3 tasks

aimeelramirez added the duplicate This issue or pull request already exists label Jan 9, 2022

final changes and checks on option 1 with testing w3c

bcb7c34

aimeelramirez changed the title ~~applying changes to footer and created a new file under social share …~~ Feature/footer social media icons - option 1 Jan 9, 2022

final changes and checks on option 1 with running lint on prettier

dcea933

aimeelramirez closed this Jan 9, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/footer social media icons - option 1 #60

Feature/footer social media icons - option 1 #60

aimeelramirez commented Jan 3, 2022

netlify bot commented Jan 3, 2022 •

edited

thescientist13 commented Jan 4, 2022

thescientist13 left a comment

thescientist13 Jan 4, 2022

thescientist13 Jan 4, 2022

thescientist13 commented Jan 4, 2022

aimeelramirez commented Jan 5, 2022

thescientist13 commented Jan 5, 2022 •

edited

Feature/footer social media icons - option 1 #60

Feature/footer social media icons - option 1 #60

Conversation

aimeelramirez commented Jan 3, 2022

Related Issue

Summary of Changes

netlify bot commented Jan 3, 2022 • edited

thescientist13 commented Jan 4, 2022

thescientist13 left a comment

Choose a reason for hiding this comment

thescientist13 Jan 4, 2022

Choose a reason for hiding this comment

thescientist13 Jan 4, 2022

Choose a reason for hiding this comment

thescientist13 commented Jan 4, 2022

aimeelramirez commented Jan 5, 2022

thescientist13 commented Jan 5, 2022 • edited

netlify bot commented Jan 3, 2022 •

edited

thescientist13 commented Jan 5, 2022 •

edited