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

feat: Handle new configuration for logos #2034

Merged
merged 1 commit into from
Nov 8, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 12 additions & 13 deletions src/components/FooterLogo/FooterLogo.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,20 +3,19 @@ import { useClient, useQuery } from 'cozy-client'
import { buildContextQuery } from 'queries'
import Divider from 'cozy-ui/transpiled/react/Divider'

import { getHomeLogos } from 'components/FooterLogo/helpers'
Copy link
Contributor

Choose a reason for hiding this comment

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

le fait d'avoir un helper permet de tester spécifiquement, cf la question sur le plantage si logos vaut un tableau vide

Copy link
Contributor

Choose a reason for hiding this comment

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

Encore faut-il que ça vaille la peine d'être testé. Ici en l'occurrence, je suis assez d'accord avec ce qui a été fait. On peut effectivement ajouter un test sur un homeLogo vide et vérifier que rien n'est affiché. AMA pas besoin de plus.


export const FooterLogo = () => {
const client = useClient()
const rootURL = client.getStackClient().uri

const contextQuery = buildContextQuery()
const { data } = useQuery(contextQuery.definition, contextQuery.options)

const logos = getHomeLogos(data, rootURL)
const logos = data?.logos?.home?.light || []
Copy link
Contributor

@JF-Cozy JF-Cozy Nov 8, 2023

Choose a reason for hiding this comment

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

on pourrait commenter ici pour indiquer que seul le theme light est supporté (car forcé). D'ailleurs, pourquoi light puisqu'on est en dark sur la home, non ? 🤔 Ca veut dire que les logos ne sont pas rangé par theme, mais par "style" ?

Copy link
Contributor

Choose a reason for hiding this comment

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

vu en zoom, ils sont bien rangés par thème 👍 donc home.light fait référence au logo pour un thème light (donc une version foncé du logo).

const secondaries = logos.filter(logos => logos.type === 'secondary')
JF-Cozy marked this conversation as resolved.
Show resolved Hide resolved
const main = logos.find(logos => logos.type === 'main')

const hasSecondaries =
logos.secondaries && Object.keys(logos.secondaries).length !== 0
const hasMain = logos.main !== undefined
const hasSecondaries = secondaries.length !== 0
const hasMain = main !== undefined

if (!hasMain && !hasSecondaries) return <div className="u-mt-3-s"></div>

Expand All @@ -27,9 +26,9 @@ export const FooterLogo = () => {
<div className="u-flex u-mh-auto u-maw-100 u-flex-items-center">
{hasMain ? (
<img
key={logos.main.url}
src={logos.main.url}
alt={logos.main.alt}
key={main.src}
src={`${rootURL}/assets${main.src}`}
alt={main.alt}
className="u-ph-1 u-pv-1 u-mah-3"
/>
) : null}
Expand All @@ -40,11 +39,11 @@ export const FooterLogo = () => {
) : null}
{hasSecondaries ? (
<div className="u-flex u-flex-grow-1 u-ov-auto u-filter-gray-100 u-pv-1">
{Object.entries(logos.secondaries).map(([logoSrc, logoAlt]) => (
{secondaries.map(({ src, alt }) => (
<img
key={logoSrc}
src={logoSrc}
alt={logoAlt}
key={src}
src={`${rootURL}/assets${src}`}
alt={alt}
className="u-ph-1 u-mah-3"
style={{
objectFit: 'contain'
Expand Down
59 changes: 36 additions & 23 deletions src/components/FooterLogo/FooterLogo.spec.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ import { CozyProvider, createMockClient } from 'cozy-client'
import { FooterLogo } from './FooterLogo'

describe('FooterLogo', () => {
const setup = ({ attributes = {} } = {}) => {
const setup = mockLogos => {
const homeLogos = mockLogos ? { logos: { home: { light: mockLogos } } } : {}
const mockClient = createMockClient({
queries: {
'io.cozy.settings/context': {
Expand All @@ -18,7 +19,7 @@ describe('FooterLogo', () => {
data: [
{
id: 'io.cozy.settings/context',
attributes
...homeLogos
}
]
}
Expand All @@ -40,42 +41,54 @@ describe('FooterLogo', () => {
})

it('should render secondaries logo only', () => {
setup({
attributes: {
home_logos: {
'/logo/1_partner.svg': 'Partner n°1',
'/logo/2_partner.svg': 'Partner n°2'
}
setup([
{
src: '/logo/partner1.png',
alt: 'Partner n°1',
type: 'secondary'
},
{
src: '/logo/partner2.png',
alt: 'Partner n°2',
type: 'secondary'
}
})
])

const images = screen.getAllByAltText(/Partner n°*?/i)
expect(images.length).toEqual(2)
})

it('should render main logo only', () => {
setup({
attributes: {
home_logos: {
'/lgoo/main_partner.svg': 'Main partner'
}
setup([
{
src: '/logo/partner_main.png',
alt: 'Main partner',
type: 'main'
}
})
])

const image = screen.getByAltText('Main partner')
expect(image).toBeInTheDocument()
})

it('should render both', () => {
setup({
attributes: {
home_logos: {
'/lgoo/main_partner.svg': 'Main partner',
'/logo/1_partner.svg': 'Partner n°1',
'/logo/2_partner.svg': 'Partner n°2'
}
setup([
{
src: '/logo/partner_main.png',
alt: 'Main partner',
type: 'main'
},
{
src: '/logo/partner1.png',
alt: 'Partner n°1',
type: 'secondary'
},
{
src: '/logo/partner2.png',
alt: 'Partner n°2',
type: 'secondary'
}
})
])

const main = screen.getByAltText('Main partner')
expect(main).toBeInTheDocument()
Expand Down
21 changes: 0 additions & 21 deletions src/components/FooterLogo/helpers.js

This file was deleted.

66 changes: 0 additions & 66 deletions src/components/FooterLogo/helpers.spec.js

This file was deleted.

Loading