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

[Enhancement] Refactor Kirby-Slide (Desktop Ready?) #1986

Closed
10 tasks
bommariusser opened this issue Jan 7, 2022 · 3 comments · Fixed by #2949
Closed
10 tasks

[Enhancement] Refactor Kirby-Slide (Desktop Ready?) #1986

bommariusser opened this issue Jan 7, 2022 · 3 comments · Fixed by #2949
Assignees
Labels
component:Slides enhancement New feature or request 🧑‍🎨 Needs UX NOT Tech refined Needs Tech kickoff - solution outlined and agreed
Milestone

Comments

@bommariusser
Copy link
Contributor

Describe the enhancement and the

Since the Ionic slide is deprecating we should consider changing our kirby slide implementation to the one Ionic is pointing towards to: https://swiperjs.com/

Also.. The implemented slide in kirby is very specific bound upon the one usecase which it is used in, but it should be more generic useable for everyone.

See also https://www.jyskebank.dk/bolig/regn-paa-bolig/beregn-laan-i-frivaerdi

Also. The slide implementation above could be more "desktop ready" with adding both button and pagination, which could be part of this issue.

Slide in general has a wide variety of usecases, see also the screenshots below..

Skærmbillede 2022-01-07 kl  12 53 09

Skærmbillede 2022-01-07 kl  12 54 02

Skærmbillede 2022-01-07 kl  12 53 52


Checklist:

The following tasks should be carried out in sequence in order to follow the process of contributing correctly.

Refinement

  • Request that the issue is UX refined; do not proceed until this is done.
  • Request that the issue is tech refined; do not proceed until this is done.

Implementation

The contributor who wants to implement this issue should:

  • Make sure you have read: "Before you get coding".
  • Signal to others you are working on the issue by assigning yourself.
  • Create a branch from the master branch following our branch naming convention.
  • Publish a WIP implementation to Github as a draft PR and ask for feedback.
  • Make sure you have implemented tests following the guidelines in: "The good: Test".
  • Update the cookbook with examples and showcases.

Review

Once the issue has been implemented and is ready for review:

  • Do a self-review.
  • Create a pull-request. If you created a draft PR during implementation you can just mark that as "ready for review".
@bommariusser bommariusser added enhancement New feature or request NOT Tech refined Needs Tech kickoff - solution outlined and agreed NOT Prioritized Issue not yet prioritized and added to a Milestone component:Slides 👶🏻 New For new issues before prioritisation and refinement labels Jan 7, 2022
@MadsBuchmann
Copy link
Contributor

Swiperjs looks good! But it has a ton of functionality some of which might be superflous.

Therefore i think it is worth considering what our exact requirements for a slider is and then discuss if it is worth dragging in another dependency to fulfill those. If the requirements are simple enough, perhaps we would be better served by building the functionality ourselves to have full control?

I've never tried building a slider from scratch so i do not know if it is a PITA, but i think it is worth discussing before jumping to the conclusion that we must use swiperjs.

@jkaltoft
Copy link
Collaborator

jkaltoft commented Feb 4, 2022

Suggestion: Do some research before "throwing in another JS library"

Swiper may or may not be overkill. Can CSS Scroll Snap cover the use cases?

@alxzak alxzak added this to Kirby Feb 10, 2022
@alxzak alxzak removed the 👶🏻 New For new issues before prioritisation and refinement label May 9, 2022
@alxzak alxzak removed the NOT Prioritized Issue not yet prioritized and added to a Milestone label May 18, 2022
@MadsBuchmann MadsBuchmann mentioned this issue Jun 13, 2022
26 tasks
@alxzak alxzak added this to the Desktop ready milestone Sep 20, 2022
@mark-drastrup mark-drastrup self-assigned this Nov 11, 2022
@mark-drastrup mark-drastrup moved this to 🚀 In Progress in Kirby Nov 11, 2022
@mark-drastrup
Copy link
Contributor

I have done some research as suggested above to see if we can implement our own slider from scratch. It hard to tell whether we can do it ourselves or not, before the slider is UX refined. If it's a simple use case like our current slider, but with pagination and buttons, then is should be feasible for us to do by ourselves. Then we could take inspiration from and expand upon a simple codepen like this: https://codepen.io/bushblade/pen/ZEpvzbK?editors=0010, which supports both desktop and mobile.
But if the expected use cases for the slider is complex, then I believe it would be easier for us to migrate our slider to use Swiper.js

Once this issue has been UX refined, then we are in a better position to decide which direction we want to go.

@mark-drastrup mark-drastrup removed their assignment Nov 14, 2022
@mark-drastrup mark-drastrup self-assigned this Mar 10, 2023
@mark-drastrup mark-drastrup moved this from 🚀 In Progress to 🔎 Review pending in Kirby Apr 25, 2023
@jakobe jakobe moved this from 🔎 Review pending to 👀 Review in progress in Kirby Jun 7, 2023
@github-project-automation github-project-automation bot moved this from 👀 Review in progress to ✅ Done in Kirby Jun 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:Slides enhancement New feature or request 🧑‍🎨 Needs UX NOT Tech refined Needs Tech kickoff - solution outlined and agreed
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

6 participants