-
Notifications
You must be signed in to change notification settings - Fork 137
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice cleanup, thanks! Left 1 nit.
src/features/sync/images/index.ts
Outdated
import RemoveIconImage from './remove_icon.svg' | ||
import DesktopImage from './desktop_icon.svg' | ||
import MobilePicture from './mobile_icon.svg' | ||
import MobilePictureImage from './mobile_picture.png' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: there'd be less confusion in this module of variable names if the urls ended in Url and the actual image elements ended in Image (or Icon or Picture I guess). But at the moment they are mixed - some urls end in Picture and some components end in Picture, etc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
k added the url
suffix and normalized them to use image
instead of icon/picture
be951d1
to
c30edea
Compare
c30edea
to
470233f
Compare
make sync images exportable
make sync images exportable
This reverts commit 2c16ae2.
This reverts commit 69ef8be.
make sync images exportable
make sync images exportable
addresses partial feedback from brave/brave-core#1019