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

Added inline style for kirby-modal-footer #1982

Merged
merged 7 commits into from
Feb 10, 2022

Conversation

NikolajSkousen
Copy link
Contributor

@NikolajSkousen NikolajSkousen commented Jan 6, 2022

Which issue does this PR close?

This PR closes #958

What is the new behavior?

Introduces a new style property on the to allow the footer to be rendered as inline. When the footer is set as inline the background-color is set to transparent and the box-shadow is removed from the footer.

Does this PR introduce a breaking change?

  • Yes
  • No

@MadsBuchmann MadsBuchmann self-assigned this Jan 10, 2022
Copy link
Contributor

@MadsBuchmann MadsBuchmann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good - i got a few comments.

@henrikvoetmand has UX reviewed it and says it is a-okay 😃

@NikolajSkousen NikolajSkousen marked this pull request as ready for review January 14, 2022 09:31
Copy link
Contributor

@MadsBuchmann MadsBuchmann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for implementing the requests! - i got two minor comments :)

describe('ModalFooterComponent inline style', () => {
it('should have a transparent background color when inline style is set', () => {
spectator = createHost(`<kirby-modal-footer [inline]="true"></kirby-modal-footer>`);
fdescribe('when inline type is set', () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like a f snuck in 🥷

Comment on lines 13 to 21
@HostBinding('class.fixed')
isFixed: boolean = true;
@HostBinding('class.inline')
@Input()
inline = false;
isInline: boolean;

@Input() set type(type: 'inline' | 'fixed') {
this.isFixed = type === 'fixed';
this.isInline = type === 'inline';
}
Copy link
Contributor

@MadsBuchmann MadsBuchmann Jan 17, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested it and it seems to work without problems! But I think it might be possible to simplify this a bit.

@HostBinding('class')
@Input() 
type: 'inline' | 'fixed' = 'fixed';

It would also match how we do it in the code otherwise 😄
Example from libs/designsystem/src/lib/components/radio/radio.component.ts ~line 19:

  @HostBinding('class')
  @Input()
  size?: 'xs' | 'sm' | 'md';

@MadsBuchmann MadsBuchmann changed the base branch from master to release/v5.0.21 February 1, 2022 07:12
@MadsBuchmann
Copy link
Contributor

I've taken the liberty of changing the base branch from master to release/v5.0.21 which contains changes for our next release 😄

Copy link
Contributor

@MadsBuchmann MadsBuchmann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - thank you for the great work 😄

@MadsBuchmann MadsBuchmann enabled auto-merge (squash) February 10, 2022 10:59
@MadsBuchmann MadsBuchmann merged commit 6828d5f into kirbydesign:main Feb 10, 2022
RasmusKjeldgaard added a commit that referenced this pull request Feb 14, 2022
* Only redraw chart if its rendered (#2017)

* Only redraw chart if its rendered

* ♻️ Spy on 'updateLabels' instead of 'updateDataLabels'

updateDataLabels was renamed to updateLabels in PR #1989.

* Updated chart property name & tests

Co-authored-by: Tobias Secher <bdytos@bankdata.dk>
Co-authored-by: Mads Buchmann Frederiksen <mbf@prochimp.dk>

* ⬆️ Bump follow-redirects from 1.13.3 to 1.14.7 (#1994)

Bumps [follow-redirects](https://github.com/follow-redirects/follow-redirects) from 1.13.3 to 1.14.7.
- [Release notes](https://github.com/follow-redirects/follow-redirects/releases)
- [Commits](follow-redirects/follow-redirects@v1.13.3...v1.14.7)

---
updated-dependencies:
- dependency-name: follow-redirects
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: RasmusKjeldgaard <rkk@bankdata.dk>

* ⬆️ Bump log4js from 6.3.0 to 6.4.0 (#2004)

Bumps [log4js](https://github.com/log4js-node/log4js-node) from 6.3.0 to 6.4.0.
- [Release notes](https://github.com/log4js-node/log4js-node/releases)
- [Changelog](https://github.com/log4js-node/log4js-node/blob/master/CHANGELOG.md)
- [Commits](log4js-node/log4js-node@v6.3.0...v6.4.0)

---
updated-dependencies:
- dependency-name: log4js
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: RasmusKjeldgaard <rkk@bankdata.dk>

* ⬆️ Bump nanoid from 3.1.22 to 3.2.0 (#2005)

Bumps [nanoid](https://github.com/ai/nanoid) from 3.1.22 to 3.2.0.
- [Release notes](https://github.com/ai/nanoid/releases)
- [Changelog](https://github.com/ai/nanoid/blob/main/CHANGELOG.md)
- [Commits](ai/nanoid@3.1.22...3.2.0)

---
updated-dependencies:
- dependency-name: nanoid
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: RasmusKjeldgaard <rkk@bankdata.dk>

* ⬆️ Bump ssri from 6.0.1 to 6.0.2 (#2027)

Bumps [ssri](https://github.com/npm/ssri) from 6.0.1 to 6.0.2.
- [Release notes](https://github.com/npm/ssri/releases)
- [Changelog](https://github.com/npm/ssri/blob/v6.0.2/CHANGELOG.md)
- [Commits](npm/ssri@v6.0.1...v6.0.2)

---
updated-dependencies:
- dependency-name: ssri
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: RasmusKjeldgaard <rkk@bankdata.dk>

* 📝 Remove grid from navigation (#2023)

This is to avoid new consumers of Kirby taking grid into use.

Co-authored-by: RasmusKjeldgaard <rkk@bankdata.dk>

* Add rounded corners and shadow to list (#2029)

* 💄 Introduce mixins for shadow and rounded corners

* 💄 Apply rounded corners and shadow to list

* 📝  Create list-experimental showcase

* 💄 Rework and rename mixins and global classes

* 💄 Enable first/last item padding in list

* 📝  Add examples for declarative list

* 📝  Refactor examples for use in showcase

* 📝  Correct attribute for section header examples

* 📝 Add examples to showcase

* ♻️ Use section instead of ion-list

Ion-list was not used for any specfic reason, and did not add
any necessary functionality, so it has been exchanged for a section
for now.

* 📝 Improve documentation

* ♻️ Rename slot for header to be more generic

* ✅ Add tests for list-experimental

* ✅  Remove focus flag from list-experimental suite

* ✅  Add tests for padding top and bottom on item

* 📝  Use correct name for slot in showcase

* 📝 Add headings to example

* ✅  Remove focus flag from item test

* 📝  Improve wording and structure of showcase

* 📝  Add multiple sections description to showcase

* ✅  Use ' instead of " in test

* Combine width style props into one in example

Co-authored-by: Jesper Kaltoft Vind <git@jesperkaltoft.dk>

* 🐛  Fix scss style for list example

* 💄 Move utility classes to own list scss module

* 💄 Apply refactored style classes to list

* ✅  Remove unnecessary wrapper div from list

Co-authored-by: Mads Buchmann Frederiksen <mbf@prochimp.dk>
Co-authored-by: Jesper Kaltoft Vind <git@jesperkaltoft.dk>

* Added inline style for kirby-modal-footer (#1982)

* updated modal-footer component ts, scss and corresponding tests. Updated showcase component.

* corrected test

* rephrased description

* Change type inline/fixed

* removed f in fdescribe, refactor of hostbinding

* 🔖Bumping version to 5.1.0 (designsystem)

Co-authored-by: Tobias Secher <77673731+bdytos@users.noreply.github.com>
Co-authored-by: Tobias Secher <bdytos@bankdata.dk>
Co-authored-by: Mads Buchmann Frederiksen <mbf@prochimp.dk>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Jesper Kaltoft Vind <git@jesperkaltoft.dk>
Co-authored-by: NikolajSkousen <85226992+NikolajSkousen@users.noreply.github.com>
@MadsBuchmann MadsBuchmann mentioned this pull request Feb 14, 2022
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Enhancement] Inline Footer
2 participants