Skip to content

[5.4] BL-12396 Guatemala branding #1 - #5972

Merged
gmartin7 merged 1 commit into
BloomBooks:Version5.4from
gmartin7:BL-12396Guatemala1
Jul 11, 2023
Merged

[5.4] BL-12396 Guatemala branding #1#5972
gmartin7 merged 1 commit into
BloomBooks:Version5.4from
gmartin7:BL-12396Guatemala1

Conversation

@gmartin7

@gmartin7 gmartin7 commented Jul 3, 2023

Copy link
Copy Markdown
Contributor
  • mostly affects Credits page
  • waiting on response from Chris

This change is Reviewable

@gmartin7
gmartin7 force-pushed the BL-12396Guatemala1 branch from 5676f86 to 1849f09 Compare July 3, 2023 21:44

@andrew-polk andrew-polk left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I know this is mostly copied from another branding/xmatter.
Anything to review in particular?

Reviewed 12 of 13 files at r1, 1 of 1 files at r2, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @gmartin7)

@gmartin7 gmartin7 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Copied some yes. We'll shortly have another branding using the same xmatter (which will get commented in the later PR w/ the second branding). Mostly review the part that has 2 logos at the top of the credits page followed by a text block under those logos.

Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @gmartin7)

@gmartin7
gmartin7 force-pushed the BL-12396Guatemala1 branch 3 times, most recently from 7e333e1 to 88900ff Compare July 4, 2023 17:57

@gmartin7 gmartin7 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Moved the comments I referred to above into this PR.

Reviewable status: 8 of 13 files reviewed, all discussions resolved (waiting on @andrew-polk)

@andrew-polk andrew-polk left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Reviewed 5 of 5 files at r3, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @gmartin7)


src/content/branding/Guatemala-RTI-GBEQT/branding.json line 32 at r3 (raw file):

            "content": "<img class='branding' src='USAID_Horiz_Spanish.svg'/>",
            "condition": "always"
        },

Is there a reason we need three slots for credits-page-branding-top? i.e. top-middle, top-left, top-right?
Can there just be a top which contains all the bits?
We're trying to just have certain slots in the xmatter which the branding can fill in. If those slots can be standard across xmatters, that's much better.
So anytime we introduce a new slot, we need to evaluate whether it is really necessary.

@gmartin7 gmartin7 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @andrew-polk)


src/content/branding/Guatemala-RTI-GBEQT/branding.json line 32 at r3 (raw file):

Previously, andrew-polk wrote…

Is there a reason we need three slots for credits-page-branding-top? i.e. top-middle, top-left, top-right?
Can there just be a top which contains all the bits?
We're trying to just have certain slots in the xmatter which the branding can fill in. If those slots can be standard across xmatters, that's much better.
So anytime we introduce a new slot, we need to evaluate whether it is really necessary.

Hmm... I'll have to think about that. It may be that I can get away with the one slot... and no new xmatter. The main reason for the extra xmatter was just to handle the extra slots, so... I'll look at that tomorrow.

@gmartin7
gmartin7 force-pushed the BL-12396Guatemala1 branch from 88900ff to 07fbfcd Compare July 5, 2023 19:31

@gmartin7 gmartin7 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Reviewable status: 7 of 13 files reviewed, 1 unresolved discussion (waiting on @andrew-polk)


src/content/branding/Guatemala-RTI-GBEQT/branding.json line 32 at r3 (raw file):

Previously, gmartin7 (Gordon Martin) wrote…

Hmm... I'll have to think about that. It may be that I can get away with the one slot... and no new xmatter. The main reason for the extra xmatter was just to handle the extra slots, so... I'll look at that tomorrow.

Rearranged things significantly. How's that?

@gmartin7
gmartin7 force-pushed the BL-12396Guatemala1 branch from 07fbfcd to f133d13 Compare July 5, 2023 20:19

@andrew-polk andrew-polk left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Reviewed 6 of 6 files at r4, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @gmartin7)


src/content/branding/Guatemala-RTI-GBEQT/branding.json line 32 at r3 (raw file):

Previously, gmartin7 (Gordon Martin) wrote…

Rearranged things significantly. How's that?

Seems to greatly simplify things. Thanks.

* mostly affects Credits page
* enlarge bottom logos
* add USAID to copyright
@gmartin7
gmartin7 force-pushed the BL-12396Guatemala1 branch from f133d13 to 902cc77 Compare July 11, 2023 20:36
@gmartin7
gmartin7 marked this pull request as ready for review July 11, 2023 20:39

@andrew-polk andrew-polk left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r5, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @gmartin7)

@gmartin7
gmartin7 merged commit b35cbe4 into BloomBooks:Version5.4 Jul 11, 2023
@gmartin7
gmartin7 deleted the BL-12396Guatemala1 branch July 11, 2023 20:55
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