Infra/assets internal new structure #3635
Conversation
… use internal assets
…in Icon and Image components
…tton, and Hint components
src/assets/internal/icons/index.js
Outdated
| @@ -1,38 +1,38 @@ | |||
| export const icons = { | |||
| get check() { | |||
| return require('./check.png'); | |||
| return {uri: require('./check.png'), width: 21, height: 15}; | |||
There was a problem hiding this comment.
Is it working?
A local assets is usually passed as a source, while external uses uri
Is that what we do in private also?
src/utils/imageUtils.ts
Outdated
| @@ -2,6 +2,23 @@ import get from 'lodash/get'; | |||
| import Assets from '../assets'; | |||
| import type {ImageSourceType} from '../components/image'; | |||
|
|
|||
| export function extractImageSource(source?: ImageSourceType) { | |||
| const sourceUri = get(source, 'uri'); | |||
There was a problem hiding this comment.
avoid using get so you'll get proper typings you can change to source?.uri
src/utils/imageUtils.ts
Outdated
|
|
||
| if (sourceUri) { | ||
| // Case when uri is a number (Local Assets) | ||
| if (typeof sourceUri === 'number') { |
There was a problem hiding this comment.
I personally don't like this solution, it changes the behavior of a local asset. You are changing the way local assets are passed which is not a good idea (for instance, you can see that ColorSwatch is broken now because it's not using our Icon component)
package.json
Outdated
| @@ -39,7 +39,8 @@ | |||
| "releaseDemo": "./scripts/release/releaseDemo.sh", | |||
| "releaseDocs": "./scripts/release/releaseDocs.sh", | |||
| "releaseEslint": "./scripts/release/releaseEslint.sh", | |||
| "releaseNative": "./scripts/release/releaseNative.sh" | |||
| "releaseNative": "./scripts/release/releaseNative.sh", | |||
| "update-web-assets": "node scripts/assets/extract-dimensions.js" | |||
There was a problem hiding this comment.
updateWebAssets - let's follow the other scripts' naming convention
scripts/assets/extract-dimensions.js
Outdated
| height: dimensions.height | ||
| }; | ||
| } catch (sizeError) { | ||
| console.log(`Error getting dimensions for ${imagePath}:`, sizeError); |
There was a problem hiding this comment.
Changed, added // eslint-disable-next-line no-restricted-syntax since we have eslint over this.
Also for the other console .
scripts/assets/extract-dimensions.js
Outdated
| return {width: 24, height: 24}; | ||
| } | ||
| } catch (error) { | ||
| console.log(`Error getting dimensions for ${imagePath}:`, error); |
scripts/assets/extract-dimensions.js
Outdated
|
|
||
| // Function to create index files with dimensions | ||
| function createIndexFile(sourcePath, targetPath, fileType) { | ||
| const files = fs.readdirSync(sourcePath).filter(file => !file.includes('@') && !file.startsWith('.')); |
There was a problem hiding this comment.
Do you know what the !file.startsWith('.') is about?
There was a problem hiding this comment.
Nope, this is Devin script.
Iv'e check it's unnecessary removed.
for now we don't want to include the "sized" files @1.5x...
scripts/assets/extract-dimensions.js
Outdated
| @@ -0,0 +1,86 @@ | |||
| const fs = require('fs'); | |||
There was a problem hiding this comment.
Let's rename the file to extractDimensions.js
Description
Based on #3618 changes, implemented new structure for local assets on web.
Assets in
.webfiles now represent in object of:{uri, width, height}, this object is valid source for RN Image.In the
uriwe will setrequire(...).defaultas value to supportwebDemoanddocuilib.webDemoseteslintinimageLoadertotrue(true by default link)ReactLiveScopeconfigurations file since it's not necessary anymore, it was a fix for images with.default- all the assets files are with.default(related PR).Snapshot version for testing:
7.39.0 - 6582Changelog
Local Assets new structure, support web and mobile devices.
Additional info
MADS-4531