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(input): add ionic theme styles and size property #29380
Open
brandyscarney
wants to merge
38
commits into
next
Choose a base branch
from
ROU-4848
base: next
Could not load branches
Branch not found: {{ refName }}
Could not load tags
Nothing to show
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Issue number: internal --------- ## What is the new behavior? Adds the initial files for the Ionic input. ## Does this introduce a breaking change? - [ ] Yes - [x] No
Issue number: internal --------- ## What is the current behavior? Input does not have a size property. ## What is the new behavior? <!-- Please describe the behavior or changes that are being added by this PR. --> - Adds the `size` property with support for the `"large"` size on the `"ionic"` theme only - Adds tests for the size property - Note: the screenshots will not look right until the fill styles are added ## Does this introduce a breaking change? - [ ] Yes - [x] No
Issue number: resolves # --------- <!-- Please do not submit updates to dependencies unless it fixes an issue. --> <!-- Please try to limit your pull request to one type (bugfix, feature, etc). Submit multiple pull requests if needed. --> ## What is the current behavior? <!-- Please describe the current behavior that you are modifying. --> ## What is the new behavior? <!-- Please describe the behavior or changes that are being added by this PR. --> - - - ## Does this introduce a breaking change? - [ ] Yes - [ ] No <!-- If this introduces a breaking change: 1. Describe the impact and migration path for existing applications below. 2. Update the BREAKING.md file with the breaking change. 3. Add "BREAKING CHANGE: [...]" to the commit description when merging. See https://github.com/ionic-team/ionic-framework/blob/main/.github/CONTRIBUTING.md#footer for more information. --> ## Other information <!-- Any other information that is important to this PR such as screenshots of how the component looks before and after the change. --> --------- Co-authored-by: Sean Perkins <13732623+sean-perkins@users.noreply.github.com> Co-authored-by: Brandy Carney <brandyscarney@users.noreply.github.com> Co-authored-by: Marcelino <brunoapmarcelino@gmail.com> Co-authored-by: Liam DeBeasi <liamdebeasi@users.noreply.github.com> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: Brandy Carney <brandy@ionic.io> Co-authored-by: Bernardo Cardoso <32780808+BenOsodrac@users.noreply.github.com>
…29268) Issue number: Internal --------- <!-- Please do not submit updates to dependencies unless it fixes an issue. --> <!-- Please try to limit your pull request to one type (bugfix, feature, etc). Submit multiple pull requests if needed. --> ## What is the new behavior? <!-- Please describe the behavior or changes that are being added by this PR. --> All changes are specific to the `ionic` theme. - Styles added for `fill="outline"` plus `labelPlacement="stacked"`. - Markup rearranged slightly to ensure label sits above outline while still being clickable to focus the input. See code comments for details. - The default `labelPlacement` is now `"stacked"`. - Values for `labelPlacement` besides `"stacked"` and `"floating"` cannot be used. Note that per the ticket, I did not account for any other scope, including styles for helper text, `labelPlacement="floating"`, `shape="round"`, etc. This means that some states will look broken for now, and will be addressed in future tickets. ## Does this introduce a breaking change? - [ ] Yes - [x] No <!-- If this introduces a breaking change: 1. Describe the impact and migration path for existing applications below. 2. Update the BREAKING.md file with the breaking change. 3. Add "BREAKING CHANGE: [...]" to the commit description when merging. See https://github.com/ionic-team/ionic-framework/blob/main/.github/CONTRIBUTING.md#footer for more information. --> ## Other information <!-- Any other information that is important to this PR such as screenshots of how the component looks before and after the change. --> --------- Co-authored-by: ionitron <hi@ionicframework.com> Co-authored-by: Maria Hutt <thetaPC@users.noreply.github.com>
Co-authored-by: Sean Perkins <13732623+sean-perkins@users.noreply.github.com>
Issue number: internal --------- <!-- Please do not submit updates to dependencies unless it fixes an issue. --> <!-- Please try to limit your pull request to one type (bugfix, feature, etc). Submit multiple pull requests if needed. --> ## What is the current behavior? The ionic input does not have a hover state. ## What is the new behavior? Adds a hover state. ## Does this introduce a breaking change? - [ ] Yes - [x] No ## Other information We do not currently have a way to test hover states. See microsoft/playwright#12077 for more information. --------- Co-authored-by: Sean Perkins <13732623+sean-perkins@users.noreply.github.com>
Co-authored-by: Sean Perkins <13732623+sean-perkins@users.noreply.github.com> Co-authored-by: ionitron <hi@ionicframework.com>
Issue number: N/A --------- <!-- Please do not submit updates to dependencies unless it fixes an issue. --> <!-- Please try to limit your pull request to one type (bugfix, feature, etc). Submit multiple pull requests if needed. --> ## What is the current behavior? <!-- Please describe the current behavior that you are modifying. --> Some of the size screenshot tests use `label-placement="floating"`. Because this label placement isn't scope we've gotten to yet, the screenshots look broken. This is okay in itself, but it causes confusion when the screenshots are updated for other unrelated features since reviewers don't expect the appearance to be off. ## What is the new behavior? <!-- Please describe the behavior or changes that are being added by this PR. --> Tests changed to use `label-placement="stacked"`, which is functionally the same as `floating` when the input has a value, but has the proper styling. ## Does this introduce a breaking change? - [ ] Yes - [x] No <!-- If this introduces a breaking change: 1. Describe the impact and migration path for existing applications below. 2. Update the BREAKING.md file with the breaking change. 3. Add "BREAKING CHANGE: [...]" to the commit description when merging. See https://github.com/ionic-team/ionic-framework/blob/main/docs/CONTRIBUTING.md#footer for more information. --> ## Other information <!-- Any other information that is important to this PR such as screenshots of how the component looks before and after the change. --> --------- Co-authored-by: ionitron <hi@ionicframework.com>
Co-authored-by: Brandy Carney <brandyscarney@users.noreply.github.com>
Issue number: Internal --------- <!-- Please do not submit updates to dependencies unless it fixes an issue. --> <!-- Please try to limit your pull request to one type (bugfix, feature, etc). Submit multiple pull requests if needed. --> ## What is the new behavior? <!-- Please describe the behavior or changes that are being added by this PR. --> Added styles for `shape="round"`, including: - Increased border radius - Increased padding when additionally using outline fill and large size - Horizontal padding also applied to label when using outline fill I also changed the `$ionic-border-radius-rounded-full` design token from 100% to 999px. `border-radius: 100%` is pretty obviously incorrect for most browsers (screenshot taken in Chrome): ![image](https://github.com/ionic-team/ionic-framework/assets/90629384/5dc47a65-0446-4e39-9ce3-1c749b6329e7) ## Does this introduce a breaking change? - [ ] Yes - [x] No <!-- If this introduces a breaking change: 1. Describe the impact and migration path for existing applications below. 2. Update the BREAKING.md file with the breaking change. 3. Add "BREAKING CHANGE: [...]" to the commit description when merging. See https://github.com/ionic-team/ionic-framework/blob/main/docs/CONTRIBUTING.md#footer for more information. --> ## Other information <!-- Any other information that is important to this PR such as screenshots of how the component looks before and after the change. --> --------- Co-authored-by: ionitron <hi@ionicframework.com>
Issue number: Internal --------- <!-- Please do not submit updates to dependencies unless it fixes an issue. --> <!-- Please try to limit your pull request to one type (bugfix, feature, etc). Submit multiple pull requests if needed. --> ## What is the new behavior? <!-- Please describe the behavior or changes that are being added by this PR. --> For the `ionic` theme: Adjusted the size of the clear button to match design specs, and updated the behavior so the button additionally only shows if the native input *or* any other actions within the `ion-input` are focused. (See code comments for details.) The actual icon used has not been modified. The ticket says to "use the material design clear icon as the reference icon for this feature," and we've already added a feature allowing the icon to be customized at the app level. Let me know if the intention was to use a different default icon with the `ionic` theme. ## Does this introduce a breaking change? - [ ] Yes - [x] No <!-- If this introduces a breaking change: 1. Describe the impact and migration path for existing applications below. 2. Update the BREAKING.md file with the breaking change. 3. Add "BREAKING CHANGE: [...]" to the commit description when merging. See https://github.com/ionic-team/ionic-framework/blob/main/docs/CONTRIBUTING.md#footer for more information. --> ## Other information <!-- Any other information that is important to this PR such as screenshots of how the component looks before and after the change. --> --------- Co-authored-by: ionitron <hi@ionicframework.com>
Co-authored-by: Amanda Johnston <90629384+amandaejohnston@users.noreply.github.com>
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
github-actions
bot
added
package: core
@ionic/core package
package: angular
@ionic/angular package
package: vue
@ionic/vue package
labels
Apr 23, 2024
brandyscarney
changed the title
feat(input): add ionic theme styles
feat(input): add ionic theme styles and size property
Apr 23, 2024
Co-authored-by: Brandy Carney <brandyscarney@users.noreply.github.com>
Issue number: internal --------- <!-- Please do not submit updates to dependencies unless it fixes an issue. --> <!-- Please try to limit your pull request to one type (bugfix, feature, etc). Submit multiple pull requests if needed. --> ## What is the current behavior? The input supports an undefined and a `round` shape. ## What is the new behavior? Adds support for the `rectangular` shape for the `ionic` theme & screenshot tests for this shape with the outline fill. ## Does this introduce a breaking change? - [ ] Yes - [x] No
Issue number: internal --------- <!-- Please do not submit updates to dependencies unless it fixes an issue. --> <!-- Please try to limit your pull request to one type (bugfix, feature, etc). Submit multiple pull requests if needed. --> ## What is the current behavior? <!-- Please describe the current behavior that you are modifying. --> The input component does not support the `soft` shape. ## What is the new behavior? <!-- Please describe the behavior or changes that are being added by this PR. --> - `soft` has been added to shape, but will only work for the `ionic` theme. - Added tests ## Does this introduce a breaking change? - [ ] Yes - [x] No <!-- If this introduces a breaking change: 1. Describe the impact and migration path for existing applications below. 2. Update the BREAKING.md file with the breaking change. 3. Add "BREAKING CHANGE: [...]" to the commit description when merging. See https://github.com/ionic-team/ionic-framework/blob/main/docs/CONTRIBUTING.md#footer for more information. --> ## Other information <!-- Any other information that is important to this PR such as screenshots of how the component looks before and after the change. --> [Preview](https://ionic-framework-git-fw-6096-ionic1.vercel.app/src/components/input/test/shape?ionic:theme=ionic)
…29482) Issue number: internal --------- ## What is the current behavior? The shape styles are being applied when the `fill` is unset in the `ionic` theme. This is incorrect per the input UX requirements as shape should only apply when `fill` is set to `"outline"`. This is made apparent when looking at the existing styles for `disabled`. ## What is the new behavior? - Only apply the shape styles to the `"outline"` fill - Move the use of the `--background` css variable to the native wrapper since this is what we use to style both the disabled and readonly states | Before | After | | ---| ---| | ![before](https://github.com/ionic-team/ionic-framework/assets/6577830/44b6e7e4-70f4-49d5-844f-a015b7284ed5) | ![after](https://github.com/ionic-team/ionic-framework/assets/6577830/06ac9431-17f4-4741-8ceb-19b40d8ba7d9) | I am not sure why this wasn't caught with the shape additions, but it's probably because the minimum pixel ratio was not met. ## Does this introduce a breaking change? - [ ] Yes - [x] No
Issue number: internal --------- ## What is the current behavior? There are no custom styles when readonly is added to an input. ## What is the new behavior? - Applies an `input-readonly` class when the `readonly` property is `true` - Styles the class in the `ionic` theme to match the design requirements for readonly inputs - Adds screenshot tests for the readonly style for an unset `fill` and `"outline"` fill ## Does this introduce a breaking change? - [ ] Yes - [x] No
Co-authored-by: ionitron <hi@ionicframework.com>
Issue number: internal --------- ## What is the current behavior? The counter text for the ionic theme does not have added styles. ## What is the new behavior? - Updates the `color` of the counter text to match the design - Updates the screenshots that were changed with this update ## Does this introduce a breaking change? - [ ] Yes - [x] No
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
package: angular
@ionic/angular package
package: core
@ionic/core package
package: vue
@ionic/vue package
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue number: internal
What is the current behavior?
The input uses
md
styles on theionic
theme.What is the new behavior?
Adds the following for the
ionic
theme:--text-color-invalid
CSS variablesize
property with support for thelarge
sizeoutline
fill styles for the inputround
shape styles for the inputfocused
,disabled
andhover
stylesDoes this introduce a breaking change?
Other information
The outline fill & stacked label were the main priority here so some screenshots may look plain. Floating label will be added in future work.