feat: add ten year map page #2317

evemartin · 2024-06-26T14:49:45Z

This change is

faucomte97

Reviewed 2 of 9 files at r1, 1 of 4 files at r2, 1 of 1 files at r3, 5 of 7 files at r4, 9 of 9 files at r5, all commit messages.
Reviewable status: all files reviewed, 10 unresolved discussions (waiting on @evemartin)

portal/urls.py line 533 at r5 (raw file):

        name="remove_fake_accounts",
    ),
    url(r"^tenYears/", ten_year_map_page, name="tenYears"),

Laura and I discussed this briefly last week, could we update tenYears to celebrate? Sounds / looks better as a URL. Means you'll have to replace it everywhere though, sorry 😬

portal/static/portal/img/howe_dell_1.png line 0 at r5 (raw file):
Pls minify this image. Example website to do it: https://tinypng.com/

portal/static/portal/img/howe_dell_2.png line 0 at r5 (raw file):
Pls minify

portal/static/portal/img/howe_dell_3.png line 0 at r5 (raw file):
Pls minify

portal/static/portal/sass/partials/_images.scss line 114 at r5 (raw file):

    top: 25.1%;
  }

There's a lot of extra whitespaces in between the different sections, please remove them 🙂

portal/templates/portal/ten_year_map.html line 54 at r5 (raw file):

                            <!-- <div class="col-sm-6 carousel-image-wrapper">
                                <img src="{% static 'portal/img/howe_dell.png' %}" alt="Illustration of person climbing steps with flag">
                            </div> -->

Remove this please

Code quote:

                            <!-- <div class="col-sm-6 carousel-image-wrapper">
                                <img src="{% static 'portal/img/howe_dell.png' %}" alt="Illustration of person climbing steps with flag">
                            </div> -->

portal/templates/portal/ten_year_map.html line 66 at r5 (raw file):

                                </div>
                            </div>
                            <div class="col-sm-6 carousel-text">

I'm assuming you've checked with Laura but I'm finding the layout of this section a bit awkward - only because the right hand side is centered and it leaves a lot of space at the top and bottom. Could the title maybe be aligned with the top of the first picture, and the Learn More button be aligned with the bottom of the second picture, and we can have the third picture be centered instead of left aligned? Not sure.

Can always leave for now though as this page is a work in progress.

portal/templates/portal/ten_year_map.html line 85 at r5 (raw file):

                                </p>
                                <div>
                                    <a href="https://www.linkedin.com/company/code-for-life-uk/posts/?feedView=all" class="button button--primary button-right-arrow">Learn more</a>

Please update this link so it opens up in a separate tab 🙏 (we normally do this with external links).

portal/templates/portal/ten_year_map.html line 108 at r5 (raw file):

                            <div class="col-sm-6 carousel-text">
                                <h5>
                                    Krakow

For future reference maybe, but can we have the accent on the "o" character for this city?

portal/static/portal/img/world_map.png line 0 at r5 (raw file):
Please delete this image if we no longer use it

evemartin

Reviewable status: 9 of 17 files reviewed, 10 unresolved discussions (waiting on @faucomte97 and @lauracumming)

portal/urls.py line 533 at r5 (raw file):

Previously, faucomte97 (Florian Aucomte) wrote…

Laura and I discussed this briefly last week, could we update tenYears to celebrate? Sounds / looks better as a URL. Means you'll have to replace it everywhere though, sorry 😬

Updated!

portal/static/portal/img/howe_dell_1.png line at r5 (raw file):

Previously, faucomte97 (Florian Aucomte) wrote…

Pls minify this image. Example website to do it: https://tinypng.com/

👍

portal/static/portal/img/howe_dell_2.png line at r5 (raw file):

Previously, faucomte97 (Florian Aucomte) wrote…

Pls minify

👍

portal/static/portal/img/howe_dell_3.png line at r5 (raw file):

Previously, faucomte97 (Florian Aucomte) wrote…

Pls minify

👍

portal/static/portal/sass/partials/_images.scss line 114 at r5 (raw file):

Previously, faucomte97 (Florian Aucomte) wrote…

There's a lot of extra whitespaces in between the different sections, please remove them 🙂

Removed :)

portal/templates/portal/ten_year_map.html line 54 at r5 (raw file):

Previously, faucomte97 (Florian Aucomte) wrote…

Remove this please

Removed, my bad 😓

portal/templates/portal/ten_year_map.html line 66 at r5 (raw file):

Previously, faucomte97 (Florian Aucomte) wrote…

I'm assuming you've checked with Laura but I'm finding the layout of this section a bit awkward - only because the right hand side is centered and it leaves a lot of space at the top and bottom. Could the title maybe be aligned with the top of the first picture, and the Learn More button be aligned with the bottom of the second picture, and we can have the third picture be centered instead of left aligned? Not sure.

Can always leave for now though as this page is a work in progress.

I actually have not checked yet, I was thinking maybe Laura could have a look as part of the current review process too - @lauracumming what are your thoughts on the layout? For now I've aligned the tops of the two columns and added a little padding between them!

portal/templates/portal/ten_year_map.html line 85 at r5 (raw file):

Previously, faucomte97 (Florian Aucomte) wrote…

Please update this link so it opens up in a separate tab 🙏 (we normally do this with external links).

Added :)

portal/templates/portal/ten_year_map.html line 108 at r5 (raw file):

Previously, faucomte97 (Florian Aucomte) wrote…

For future reference maybe, but can we have the accent on the "o" character for this city?

Added!

portal/static/portal/img/world_map.png line at r5 (raw file):

Previously, faucomte97 (Florian Aucomte) wrote…

Please delete this image if we no longer use it

👍

evemartin

Reviewable status: 9 of 17 files reviewed, 10 unresolved discussions (waiting on @faucomte97 and @lauracumming)

portal/templates/portal/ten_year_map.html line 66 at r5 (raw file):

Previously, evemartin wrote…

I actually have not checked yet, I was thinking maybe Laura could have a look as part of the current review process too - @lauracumming what are your thoughts on the layout? For now I've aligned the tops of the two columns and added a little padding between them!

faucomte97

Reviewed 7 of 8 files at r6, all commit messages.
Reviewable status: 16 of 17 files reviewed, 1 unresolved discussion (waiting on @lauracumming)

faucomte97

Reviewed 2 of 2 files at r7, all commit messages.
Reviewable status: all files reviewed, 6 unresolved discussions (waiting on @evemartin and @lauracumming)

portal/templates/portal/ten_year_map.html line 55 at r7 (raw file):

                                <div class="col-sm-6">
                                    <div class="row carousel-image-wrapper">
                                        <img src="{% static 'portal/img/howe_dell_1.png' %}" style="padding-right:20px" alt="Code for Life volunteers outside Howe Dell Primary School">

Move to CSS

portal/templates/portal/ten_year_map.html line 57 at r7 (raw file):

                                        <img src="{% static 'portal/img/howe_dell_1.png' %}" style="padding-right:20px" alt="Code for Life volunteers outside Howe Dell Primary School">
                                    </div>
                                    <div class="row carousel-image-wrapper" style="margin-top:15px;padding-right:20px">

Move to CSS

portal/templates/portal/ten_year_map.html line 62 at r7 (raw file):

                                </div>
                                <div class="col-sm-6 carousel-text">
                                    <h4 style="margin-top:0rem !important;">

Move to CSS

portal/templates/portal/ten_year_map.html line 79 at r7 (raw file):

                                        selected.
                                    </p>
                                    <div style="margin-top:2rem;">

Move to CS

portal/templates/portal/ten_year_map.html line 85 at r7 (raw file):

                            </div>
                            <div class="row">
                                <div class="row carousel-image-wrapper" style="margin-top:15px;padding-left:15px; padding-right:15px">

Remove row class, move top margin to CSS

evemartin

Reviewable status: 15 of 17 files reviewed, 5 unresolved discussions (waiting on @faucomte97)

portal/templates/portal/ten_year_map.html line 55 at r7 (raw file):

Previously, faucomte97 (Florian Aucomte) wrote…

Move to CSS

Moved!

portal/templates/portal/ten_year_map.html line 57 at r7 (raw file):

Previously, faucomte97 (Florian Aucomte) wrote…

Move to CSS

Moved!

portal/templates/portal/ten_year_map.html line 62 at r7 (raw file):

Previously, faucomte97 (Florian Aucomte) wrote…

Move to CSS

Moved!

portal/templates/portal/ten_year_map.html line 79 at r7 (raw file):

Previously, faucomte97 (Florian Aucomte) wrote…

Move to CS

Moved!

portal/templates/portal/ten_year_map.html line 85 at r7 (raw file):

Previously, faucomte97 (Florian Aucomte) wrote…

Remove row class, move top margin to CSS

Moved!

faucomte97

Reviewed all commit messages.
Reviewable status: 15 of 17 files reviewed, 2 unresolved discussions (waiting on @evemartin)

portal/static/portal/sass/partials/_images.scss line 96 at r8 (raw file):

.ten-year-carousel-image--column {
  padding-right: 20px;
}

Remove ten-year from these class names - in essence we're defining styles for a carousel, regardless of what the content for it is.

Code quote:

.ten-year-button {
  margin-top: 2rem;
}

.ten-year-carousel-image {
  margin-top: 15px;
}

.ten-year-carousel-image--column {
  padding-right: 20px;
}

portal/templates/portal/ten_year_map.html line 62 at r7 (raw file):

Previously, evemartin wrote…

Moved!

Unfortunately the heading is no longer top aligned with the image 😕

faucomte97

Reviewed 2 of 2 files at r8.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @evemartin)

evemartin

Reviewable status: 14 of 17 files reviewed, 2 unresolved discussions (waiting on @faucomte97)

portal/static/portal/sass/partials/_images.scss line 96 at r8 (raw file):

Previously, faucomte97 (Florian Aucomte) wrote…

Remove ten-year from these class names - in essence we're defining styles for a carousel, regardless of what the content for it is.

Good point, fixed!

portal/templates/portal/ten_year_map.html line 62 at r7 (raw file):

Previously, faucomte97 (Florian Aucomte) wrote…

Unfortunately the heading is no longer top aligned with the image 😕

Ahh I thought I could do it by getting rid of the styling since removing the rows helped with the padding/margins, but it looks like not - styling should be added back now in classes!

faucomte97

Reviewed 3 of 3 files at r9, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @evemartin)

evemartin and others added 7 commits June 25, 2024 12:39

start work

f6841f5

add pins to map

91de495

update ten year map page

06d2282

update position of pins

3f1fa4e

address PR comments

0a7a757

Merge master

ddcf762

Clean up and refactor a few things

aafb12c

SKairinos closed this Jul 12, 2024

SKairinos reopened this Jul 12, 2024

evemartin and others added 6 commits July 16, 2024 11:10

add hatfield info

bbb8163

adjust pins on screen resize

02ccad7

Merge branch 'master' into add-ten-year-map-page

3605ca4

remove unused code, add test for ten year map page

89b4b8b

update hatfield info

adb2d47

Merge branch 'master' into add-ten-year-map-page

d49a38f

faucomte97 requested changes Jul 17, 2024

View reviewed changes

address PR comments

f67ced6

evemartin commented Jul 17, 2024

View reviewed changes

faucomte97 reviewed Jul 17, 2024

View reviewed changes

adjust third image, fix carousel

f7eb49f

faucomte97 requested changes Jul 18, 2024

View reviewed changes

address PR comments related to tidying CSS

9b8e826

evemartin commented Jul 18, 2024

View reviewed changes

faucomte97 requested changes Jul 18, 2024

View reviewed changes

faucomte97 reviewed Jul 18, 2024

View reviewed changes

addressed PR comments

00e395f

evemartin commented Jul 18, 2024

View reviewed changes

faucomte97 approved these changes Jul 18, 2024

View reviewed changes

evemartin merged commit 9c7c69d into master Jul 18, 2024
7 checks passed

evemartin deleted the add-ten-year-map-page branch July 18, 2024 11:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add ten year map page #2317

feat: add ten year map page #2317

evemartin commented Jun 26, 2024 •

edited by faucomte97

Loading

faucomte97 left a comment

evemartin left a comment

evemartin left a comment

faucomte97 left a comment

faucomte97 left a comment

evemartin left a comment

faucomte97 left a comment

faucomte97 left a comment

evemartin left a comment

faucomte97 left a comment

feat: add ten year map page #2317

feat: add ten year map page #2317

Conversation

evemartin commented Jun 26, 2024 • edited by faucomte97 Loading

faucomte97 left a comment

Choose a reason for hiding this comment

evemartin left a comment

Choose a reason for hiding this comment

evemartin left a comment

Choose a reason for hiding this comment

faucomte97 left a comment

Choose a reason for hiding this comment

faucomte97 left a comment

Choose a reason for hiding this comment

evemartin left a comment

Choose a reason for hiding this comment

faucomte97 left a comment

Choose a reason for hiding this comment

faucomte97 left a comment

Choose a reason for hiding this comment

evemartin left a comment

Choose a reason for hiding this comment

faucomte97 left a comment

Choose a reason for hiding this comment

evemartin commented Jun 26, 2024 •

edited by faucomte97

Loading