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

Default value for fit property set to cover #1304

Closed
wants to merge 2 commits into from

Conversation

harshdoesdev
Copy link
Contributor

Default fit property if both width and height are defined

Example:

-- ftd.column:
padding.rem: 2
spacing.fixed.rem: 1

;; no default value will be assigned to `fit`

-- ftd.text: 1. no default value will be assigned to `fit`

-- ftd.image:
src: https://picsum.photos/800/600




;; no default value will be assigned to `fit` if just width is defined

-- ftd.text: 2. no default value will be assigned to `fit` if just width is defined

-- ftd.image:
src: https://picsum.photos/800/600
width.fixed.px: 300




;; no default value will be assigned to `fit` if just height is defined

-- ftd.text: 3. no default value will be assigned to `fit` if just height is defined

-- ftd.image:
src: https://picsum.photos/800/600
height.fixed.px: 350




;; `fit` will be set to `cover`

-- ftd.text: 4. `fit` will be set to `cover`

-- ftd.image:
src: https://picsum.photos/800/600
width.fixed.px: 300
height.fixed.px: 350



;; user defined `fit` property will override the default one

-- ftd.text: 5. user defined `fit` property will override the default one

-- ftd.image:
src: https://picsum.photos/800/600
width.fixed.px: 300
height.fixed.px: 350
fit: contain

-- end: ftd.column

Output:

_Users_harshsingh_Documents_Projects_NewSpace_fastn_ftd_t_js_59-image-fit-cover-when-width-height-defined manual html

@amitu
Copy link
Contributor

amitu commented Sep 13, 2023

@harshdoesdev I do not understand why are you implementing Default fit property if both width and height are defined and not default value of cover is fit. Is there any harm in using fit if both are not specified? If neither is specified?

If there is no harm the implement the simpler logic. If there is the harm then demonstrate the harm first.

@harshdoesdev harshdoesdev added the In Progress Development is in progress label Sep 13, 2023
@harshdoesdev harshdoesdev reopened this Sep 13, 2023
@harshdoesdev harshdoesdev changed the title Set Default fit property if both width and height are defined Default value for fit property set to cover Sep 14, 2023
@Heulitig
Copy link
Contributor

This has unexpected behavior in certain images used in fastn.com and their invocation parameters will have to be changed to make those look unchanged. So closing this PR (as it could affect UI behavior in already used ftd.image's in fastn packages).

@Heulitig Heulitig closed this Sep 20, 2023
@amitu amitu deleted the image-fit-default branch September 20, 2023 10:19
@amitu
Copy link
Contributor

amitu commented Sep 20, 2023

@harshdoesdev let's do this when we have edition feature implemented, and we create our next edition. I have added it to list of things we want to do on next edition: https://fastn.com/d/next-edition/.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
In Progress Development is in progress
Development

Successfully merging this pull request may close these issues.

3 participants