#635: Fixed Footer UI for smaller devices #636

imshrishk · 2025-01-12T16:16:11Z

Made the UI better for the footer in smaller devices.

quozl

Thanks. Welcome, and good try.

But overall poor work; you don't seem to have understood the changes you made, as if you didn't look at git diff or didn't use git add --interactive tools available to you. You also slipped in many other changes that aren't in your commit message or pull request comments.

don't leave instructions from your teacher, mentor or LLM in the changes,
restrict your edits and your tools to only make the changes needed,
separate indentation edits into another pull request (if you're new), or another commit (if you're a regular contributor),
don't remove comments from HTML without explaining why; we have them there for reasons, in particular so that people viewing source on a web browser can easily find the elements in use,
use our guide to making commits checklist for writing commit messages; several of the points in that guide were not followed,
end the commit message with "fixes #N" if the commit is to fix an issue; it may be that an issue is your intended target for these commits, but as you didn't link them you leave it to others to figure out your intent,

You may have arrived here without discovering our working style.

Contributors who hope to make a pull request should review other pull requests, starting with any that are open, but also any they haven't seen in the recently closed. I think if you did that, you'd have scored higher.

Hope that helps!

quozl · 2025-01-12T20:15:17Z

_includes/footer.html

@@ -1,5 +1,5 @@
 <!-- Footer -->
-<footer  style="background-color:#282828;padding-top:5px;">
+<footer style="background-color:#282828;padding-top:5px;">


Don't make unnecessary changes like this one.

quozl · 2025-01-12T20:15:27Z

_includes/footer.html

@@ -9,9 +9,10 @@ <h5 style="text-align:right;margin-right:20px;">
    </h5>
    <div class="container">
        <div class="row">
-            <div class="col-md-12" >
+            <div class="col-md-12">


Unnecessary change.

quozl · 2025-01-12T20:15:49Z

_includes/footer.html

                <div class="row justify-content-around">
-                    <div class="col-md-2 widget">
+                    <!-- Donation Info Section -->


Deletion of our comments; which is an unecessary change.

quozl · 2025-01-12T20:16:08Z

_includes/footer.html

@@ -20,66 +21,72 @@ <h6 class="footerHeaderStyle align-center-md text-center-md">
                            </p>
                        </h6>
                    </div>
-                    <div class="col-md-3 widget">
+
+                    <!-- Sugar Labs Section -->


Deletion of our comments.

quozl · 2025-01-12T20:16:41Z

_includes/footer.html

-				<li class="footerListStyle"><a href="{{ site.baseurl }}/leadership">Leadership and Governance</a></li>
+                                <li class="footerListStyle"><a href="https://wiki.sugarlabs.org/go/Events#Upcoming_Events">Upcoming Events</a></li>-->
+                                <li class="footerListStyle"><a href="{{ site.baseurl }}/donate">Donate to Sugar Labs</a></li>
+                                <li class="footerListStyle"><a href="{{ site.baseurl }}/leadership">Leadership and Governance</a></li>


Deletion of our comments.

quozl · 2025-01-12T20:22:05Z

_includes/footer.html

@@ -108,20 +115,90 @@ <h4 class="text-center"><a href="https://github.com/sugarlabs/www"><b><span clas
    </div>
 </footer>

+<!-- Add this CSS to your existing stylesheet or in a style tag -->


Comment not understood; if these were directions to you by another, such as an LLM, then it is poor form to include them here.

quozl · 2025-01-12T20:22:33Z

_includes/footer.html

-<script  type="text/javascript" src="{{ site.baseurl }}/js/plugins.js"></script>
-<script  type="text/javascript" src="{{ site.baseurl }}/js/owl.carousel.min.js"></script>
+<script type="text/javascript" src="{{ site.baseurl }}/js/plugins.js"></script>
+<script type="text/javascript" src="{{ site.baseurl }}/js/owl.carousel.min.js"></script>


Unnecessary change.

quozl · 2025-01-12T20:22:52Z

_includes/footer.html

 <script type="text/javascript">
 if (typeof jQuery.waypoints == 'undefined') {
    document.write('<script src="{{ site.baseurl }}/js/waypoints.js"><\/script>');
 }
 </script>

 <!-- Inline Javascript scripts -->
-<script  type="text/javascript">
+<script type="text/javascript">


Unnecessary change.

quozl · 2025-01-12T20:23:36Z

_includes/footer.html

    $(document).on('click', "a[href='#top'], #scrollToTopBtn", function (e) {
-    $('html, body').animate({ scrollTop: 0 }, 'slow');
-    return false;
+        $('html, body').animate({ scrollTop: 0 }, 'slow');


While indentation changes are helpful, there are so many in this pull request that it hides the change you are asking for review. It would be common to use a separate pull request for such changes.

quozl · 2025-01-12T20:24:01Z

_includes/footer.html

    (function(i,s,o,g,r,a,m){i['GoogleAnalyticsObject']=r;i[r]=i[r]||function(){
    (i[r].q=i[r].q||[]).push(arguments)},i[r].l=1*new Date();a=s.createElement(o),
    m=s.getElementsByTagName(o)[0];a.async=1;a.src=g;m.parentNode.insertBefore(a,m)
    })(window,document,'script','https://www.google-analytics.com/analytics.js','ga');
    ga('create', 'UA-102776855-1', 'auto');
    ga('send', 'pageview');
-</script>
+</script>


You have removed the newline at end of file. Don't.

imshrishk · 2025-01-13T08:39:28Z

Thanks a lot for the constructive feedback. I will work on it and try to fix my errors.

quozl · 2025-01-13T08:53:21Z

No worries, looking forward to your changes to this pull request.

quozl · 2025-01-13T20:17:37Z

Note that 7cb8783 has included merge conflict markers from git.

carefully observe when git has reported a merge conflict,
use conflict resolution tools and techniques,
if not possible to resolve conflict within the time you have, use the git tools for abandoning the merge,
use git diff or git add --interactive to observe merge conflict markers,
carefully observe changes made to files before git commit,
avoid git add --all or git commit --all if you have done any merging.

imshrishk · 2025-01-14T15:24:32Z

I was having issues fixing this merge conflict. To make up for this I have made another PR that fixes the same issue. Hope it is alright to do so.

imshrishk added 2 commits January 12, 2025 21:38

Fixed Footer UI for smaller devices

4578c88

Update footer.html

a87f81f

quozl requested changes Jan 12, 2025

View reviewed changes

quozl mentioned this pull request Jan 12, 2025

Footer not responsive/UI of footer not good on smaller screens #635

Open

imshrishk marked this pull request as draft January 12, 2025 23:03

Fixed sugarlabs#635

7cb8783

imshrishk requested a review from quozl January 13, 2025 14:59

imshrishk marked this pull request as ready for review January 13, 2025 17:42

Update footer.html

cc40e01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

#635: Fixed Footer UI for smaller devices #636

#635: Fixed Footer UI for smaller devices #636

imshrishk commented Jan 12, 2025

quozl left a comment

quozl Jan 12, 2025

quozl Jan 12, 2025

quozl Jan 12, 2025

quozl Jan 12, 2025

quozl Jan 12, 2025

quozl Jan 12, 2025

quozl Jan 12, 2025

quozl Jan 12, 2025

quozl Jan 12, 2025

quozl Jan 12, 2025

imshrishk commented Jan 13, 2025

quozl commented Jan 13, 2025

quozl commented Jan 13, 2025

imshrishk commented Jan 14, 2025

#635: Fixed Footer UI for smaller devices #636

Are you sure you want to change the base?

#635: Fixed Footer UI for smaller devices #636

Conversation

imshrishk commented Jan 12, 2025

quozl left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

imshrishk commented Jan 13, 2025

quozl commented Jan 13, 2025

quozl commented Jan 13, 2025

imshrishk commented Jan 14, 2025