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

calcite-modal's scale overrides calcite-input's scale #6494

Closed
omarkawach opened this issue Feb 16, 2023 · 5 comments
Closed

calcite-modal's scale overrides calcite-input's scale #6494

omarkawach opened this issue Feb 16, 2023 · 5 comments
Assignees
Labels
4 - verified Issues that have been released and confirmed resolved. ArcGIS Online Issues logged by ArcGIS Online team members. bug Bug reports for broken functionality. Issues should include a reproduction of the bug. estimate - 2 Small fix or update, may require updates to tests. has workaround Issues have a workaround available in the meantime. p - high Issue should be addressed in the current milestone, impacts component or core functionality

Comments

@omarkawach
Copy link

Actual Behavior

When calcite-input gets loaded by the lazy loader (the first time), it disregards its own scale and searches the ancestry for a scale.

In the case of a modal set to scale="l", the calcite-input will find the modal and use it for its scale even though the calcite-input was set to scale "m".

Our subsequent calcite-input instances are working correctly in the Arcade editor

Expected Behavior

The scale of calcite-input should not be overwritten by another component when a scale is already set

<calcite-input
      value={""}
      icon="magnifying-glass"
      clearable
      scale="s"
/>

Reproduction Sample

https://github.com/Esri/calcite-components/blob/cad286dd84163d61aae15c55dac3c01808aea221/src/components/input/input.tsx#L452

Reproduction Steps

Sharing the problematic line of code. Talk to @jcfranco

Difficult to reproduce in a codepen https://codepen.io/omarkawach/pen/VwGYMmo

Reproduction Version

1.0.7

Relevant Info

No response

Regression?

No response

Impact

We had to do a workaround for now

<div scale="m"><calcite-input ... /></div>

This way calcite input finds an ancestor with a scale other than the modal's scale.

Esri team

ArcGIS Online

@omarkawach omarkawach added 0 - new New issues that need assignment. bug Bug reports for broken functionality. Issues should include a reproduction of the bug. needs triage Planning workflow - pending design/dev review. labels Feb 16, 2023
@github-actions github-actions bot added the ArcGIS Online Issues logged by ArcGIS Online team members. label Feb 16, 2023
@jcfranco jcfranco added the has workaround Issues have a workaround available in the meantime. label Mar 30, 2023
@geospatialem geospatialem added 1 - assigned Issues that are assigned to a sprint and a team member. and removed 0 - new New issues that need assignment. needs triage Planning workflow - pending design/dev review. labels Mar 30, 2023
@geospatialem geospatialem added p - high Issue should be addressed in the current milestone, impacts component or core functionality estimate - 2 Small fix or update, may require updates to tests. labels Mar 30, 2023
@jcfranco
Copy link
Member

This is legacy behavior that snuck into 1.0.0, and can have unintended consequences if component ancestors have scale or status set.

Related issues:

@jcfranco jcfranco added 2 - in development Issues that are actively being worked on. and removed 1 - assigned Issues that are assigned to a sprint and a team member. labels Mar 30, 2023
jcfranco added a commit that referenced this issue Apr 5, 2023
… prevent components from unintentionally picking up a different scale/status value from an ancestor (#6506)

**Related Issue:** #6494

## Summary

This PR updates remaining non-children components to no longer look up
their ancestry to set props.

---------

Co-authored-by: Ali Stump <[email protected]>
@jcfranco jcfranco added 3 - installed Issues that have been merged to master branch and are ready for final confirmation. and removed 2 - in development Issues that are actively being worked on. labels Apr 5, 2023
@github-actions github-actions bot assigned geospatialem and unassigned jcfranco Apr 5, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Apr 5, 2023

Installed and assigned for verification.

@geospatialem
Copy link
Member

@omarkawach Wasn't able to repro the original issue in the Codepen to verify if the fix is working now. Could you verify if the Arcade editor is working as-expected with 1.3.0-next.4?

@omarkawach
Copy link
Author

Verified the fix in the editor 🙂

I will reopen this issue if the problem comes up again.

@geospatialem geospatialem removed the 3 - installed Issues that have been merged to master branch and are ready for final confirmation. label Apr 18, 2023
@geospatialem geospatialem added the 4 - verified Issues that have been released and confirmed resolved. label Apr 18, 2023
@geospatialem
Copy link
Member

Awesome, thank you @omarkawach! Marking this as verified and closing it out for the upcoming April release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4 - verified Issues that have been released and confirmed resolved. ArcGIS Online Issues logged by ArcGIS Online team members. bug Bug reports for broken functionality. Issues should include a reproduction of the bug. estimate - 2 Small fix or update, may require updates to tests. has workaround Issues have a workaround available in the meantime. p - high Issue should be addressed in the current milestone, impacts component or core functionality
Projects
None yet
Development

No branches or pull requests

4 participants