-
Notifications
You must be signed in to change notification settings - Fork 23
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
Add rounded corners and shadow to list #2029
Add rounded corners and shadow to list #2029
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good work! 💪🏻
In addition to the code comments I have a question/suggestion/request/wet dream/topic for discussion:
Can we use "proper" markup for lists? Is it possible to make kirby components for lists use e.g. <ul>
and <li>
under the hood?
libs/designsystem/src/lib/components/list/list-experimental/list-experimental.component.html
Outdated
Show resolved
Hide resolved
libs/designsystem/src/lib/components/list/list-experimental/list-experimental.component.scss
Outdated
Show resolved
Hide resolved
...cookbook/src/app/examples/list-experimental-example/list-experimental-example.component.scss
Outdated
Show resolved
Hide resolved
Would be great, but that would be a major overhaul - and seems to be well out of scope of this issue. The list could be refactored to utilize |
One is allowed to dream... 😁 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A final (short) discussion about globally available "CSS classes" and we're good to go 🚀
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.
Co-authored-by: Jesper Kaltoft Vind <git@jesperkaltoft.dk>
b534786
to
a56f150
Compare
* 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>
Which issue does this PR close?
This PR closes #1814
What is the new behavior?
The list experimental component has been changed, so content is now projected into a list-content wrapper, that has rounded corners and shadow. This way, we do not need very specific styling rules for rounding the corners of the first and last item in the list, which has proven to be error-prone. Now, it is simply the container (the list) that is responsible for this styling.
To simplify the list a great deal, there is no longer the concept of nested sections within the list. Instead, multiple lists should just be added one after another, optionally using the section-header component for dividing the lists with headers.
To document these changes, the showcase for the list in general had to be created, so this PR also contains some unrelated showcase code.
Note: The ion-list component was not used for anything in this regard, and has therefore been refactored to use a
<section>
instead. Later, if the ion-list specific code is needed, it can be added back with very little work, if any.Does this PR introduce a breaking change?
Checklist:
The following tasks should be carried out in sequence in order to follow the process of contributing correctly.
Reminders
Review
When the pull request has been approved it will be automatically merged to master via automerge.