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

Migrate to version 7 of a11y-dialog #28

Closed
renestalder opened this issue Apr 29, 2021 · 13 comments
Closed

Migrate to version 7 of a11y-dialog #28

renestalder opened this issue Apr 29, 2021 · 13 comments

Comments

@renestalder
Copy link

Version 7 is out and based on the changelog, adds some fundamental improvement regarding the fallback to the native dialog element (by removing it).

(btw. the README currently mentions that the vue wrapper is based on version 5 of the a11y-dialog but seems the code uses v6 already)

@renestalder renestalder changed the title MIgrate to version 7 of a11y-dialog Migrate to version 7 of a11y-dialog Apr 29, 2021
@roblevintennis
Copy link
Contributor

roblevintennis commented Jan 10, 2022

Hello @morkro I have had a dialogue with Kitty to ask for recommendations on the Vue port of a11y-dialog. I'm the author of AgnosticUI and I'm very motivated to use the wonderful a11y-dialog to support all versions of our dialogs (which means I will need to create a Svelte and Angular port as well; exciting I think as it will mean "no excuses" for folks not to use the battle-tested a11y-dialog to create a11y capable dialogs!)

I have temporarily made a proof-of-concept at https://github.com/AgnosticUI/vue3-a11y-dialog which ideally gets completely deleted and properly ported back here to your repository so we only have one Vue repo folks have to look for 😄

I would have started this discussion on its own thread but this one seems pretty relevant and there's no Discussions tab that I can see on my end. I hope you don't mind me piggybacking @renestalder

Some items that could use feedback regarding the vue 3 / v7 compatible refactor:

  1. I've used Vue 3 + Vite as that seems to be the current direction in the Vue community (and I enjoy Vite's speed so much)
  2. I currently have no tests — I see a Vitetest but cannot get it working yet and I believe it's too early to use that. Jest and Vite feel like an odd and challenging combination. How would we feel if I leveraged Cypress and replicated Kitty's e2e tests in a11y-dialog? I may be able to even use those as a parity baseline not sure without trying. But would you be ok if we didn't have unit but only e2e? I think this would be easiest without any suffering to the coverage and in fact more. Thoughts?

UPDATE: I've been spiking on this and it occurs to me it would be silly to completely retest a11y-dialog for a wrapper as that's really a test smell (testing an external library that should [and does!] have it's own tests).

I think just some minimal verification that the modal shows, can be closed, has correct structure, etc., is enough.

  1. Docs. Should I just replicate the style you have on the current vue-a11y-dialog and thus update the README.md with new examples and props that still exist?
  2. Branching / Versioning — my version is definitely only Vue 3 and so I intuitively predict only 20-30% will want to use it initially. Should I just fork your repo and we can use a next branch for the moment? Should I do a major semantic bump on the package.json? Please advise what works best for you.

I'm excited to help out here and continue to grow the usage of a11y-dialog and thus accessible dialogs in the wild!

P.S. I found vue-a11y-dialog-next and the npm too and it was a bit confusing to me because from what I see work hasn't started on that yet (I could be wrong). I can ping the author once we get further if that makes sense.

@KittyGiraudel
Copy link
Contributor

KittyGiraudel commented Jan 10, 2022

I have had a dialogue with Kitty

Badum tss! 🥁

Branching / Versioning — my version is definitely only Vue 3 and so I intuitively predict only 20-30% will want to use it initially. Should I just fork your repo and we can use a next branch for the moment? Should I do a major semantic bump on the package.json? Please advise what works best for you.

Depending on which version of Vue is most popular (3 I reckon?), you could also migrate main to a vue-2 branch, and use main for the latest version (for Vue 3).

@KittyGiraudel
Copy link
Contributor

Also taking the liberty to ping @marcus-herrmann here, since he apparently attempted a Vue 3 fork about a year ago, as raised by @roblevintennis.

@marcus-herrmann
Copy link
Contributor

@KittyGiraudel Yes, vue-a11y-dialog-next is a fork/proof of concept, but incomplete (tests are lacking). Back then I also used Vite, same as @roblevintennis. It is really absurd, but after releasing my ebook, I haven't had any Vue/A11y projects, neither customer stuff nor side projects.

@roblevintennis
Copy link
Contributor

roblevintennis commented Jan 10, 2022

Hello Kitty (why is at mention not working for me with at-Kitty 🤔 ), thanks for coordinating things! I like the idea of migrating main to vue2.

@marcus-herrmann I'm just connecting the dots — funny, I had https://accessible-vue.com/chapter/4/ opened in a browser tab too!

Funny back story to all this I was first considering implementing the dialog JavaScript in AgnosticUI and did the usual bunch of research assembling a huge list of about dozen requirements. I was quite bummed to proceed (especially since I would have to do in React, Svelte, Vue 3, and Angular!); I mean, how unlikely that I could get it correct without many bugs right!?

I'm very glad I revisited just wrapping Kitty's wonderful and battle-tested a11y-dialog. I have a feeling seeing you recommend this in your ebook might have further persuaded me 💥 💯 I've already wrapped in React and it was a breeze.

after releasing my ebook, I haven't had any Vue/A11y projects, neither customer stuff nor side projects

That IS absurd! Dang. Well hopefully that changes soon!

I'm looking forward to seeing where this all goes and hopefully all of us collaborating on moving this project across more popular frameworks! Maybe in a small way it helps bring a11y awareness — especially if efforts result in making it easy for junior FE devs to "just do the right thing"!

@roblevintennis
Copy link
Contributor

I've added Cypress tests tonight and moved the work to a proper fork at https://github.com/AgnosticUI/vue-a11y-dialog/tree/vue-3 (so the vue-3 branch for time-being).

I think the outstanding things left that come to mind are:

  • Linting
  • Verification of the dist files (I tried to stay close to the rollup options from before but it's using Vite build.lib with a rollup section per the Vite doc recommendations). It builds umd and es modules.
  • Peer review and decision on preferred branching / versioning strategy

@morkro
Copy link
Owner

morkro commented Jan 11, 2022

Thanks @roblevintennis, @marcus-herrmann, and also @KittyGiraudel for all the energy and effort you've put into this so far. I really appreciate it!

First and foremost, bringing vue-a11y-dialog up-to-date with the latest a11y-dialog version is long overdue and I apologise for letting this sit for so long. 🥲

To address some of your comments:

@roblevintennis I've used Vue 3 + Vite as that seems to be the current direction in the Vue community (and I enjoy Vite's speed so much). [...] Branching / Versioning — my version is definitely only Vue 3 and so I intuitively predict only 20-30% will want to use it initially. Should I just fork your repo and we can use a next branch for the moment? Should I do a major semantic bump on the package.json? Please advise what works best for you.
@KittyGiraudel Depending on which version of Vue is most popular (3 I reckon?), you could also migrate main to a vue-2 branch, and use main for the latest version (for Vue 3).

I'd intended the main branch to contain the latest, recommended Vue version and for Vue 2 users could simply point to vue-a11y-dialog@0.5.2. This is how other library authors have done the Vue 1 to 2 version upgrade and I like that approach. In case of updating the main branch to the latest a11y-dialog version, I would consider this to be a minor update and not a major, hence be 1.1.0. There won't be a fundamental rewrite for users of the library after an update. (Correct me if I am wrong though)

@roblevintennis I've been spiking on this and it occurs to me it would be silly to completely retest a11y-dialog for a wrapper as that's really a test smell (testing an external library that should [and does!] have it's own tests).

I think just some minimal verification that the modal shows, can be closed, has correct structure, etc., is enough.

Yes, I agree with that. In my opinion, we should only test the Vue-specific wrapper implementation, not a11y-dialog business logic.

@roblevintennis Docs. Should I just replicate the style you have on the current vue-a11y-dialog and thus update the README.md with new examples and props that still exist?

Sounds good to me :)

@morkro
Copy link
Owner

morkro commented Jan 11, 2022

On second thought, I now understand the Vue 2/3 discussion better and realise the need to support the latest a11y-dialog version for Vue 2 version of this wrapper. In this case, I see the following options:

  • Continue increasing the previous version to 0.6.0 which points to Vue 2, meaning everything lower than 1.0.0 is Vue 2 (potentially confusing though)
  • Introduce a separate branch that points to Vue 2 implementation, e.g. legacy@1.0.0
  • Don't support Vue 2

What does everyone prefer?

@renestalder
Copy link
Author

@morkro Do you think the hard split is necessary, or can you possibly release everything under the same versions always via vue-demi?

Otherwise, the versioning schema you suggest for sure works as it was never raised to 1.0.0 before, so you could even make breaking changes in the Vue 2 version. But as you say, it will get confusing at some point as you will have this wild mix and your changelogs and the version history, that I would say, you're better off dropping Vue 2 support at some point if it doesn't work via something like vue-demi.

Furthermore, I thought to have read that Vue 2.7 will be the last minor release for v2, but can't find the information anymore.

@roblevintennis
Copy link
Contributor

roblevintennis commented Jan 11, 2022

Hi @morkro — thanks for the input and clarifications!

Continue increasing the previous version to 0.6.0 which points to Vue 2

I think this makes sense; partially because of how it's already sort of the reality with:

If you still need to support Vue 2, you can stay on version [0.5.2]

I think you could say that these versions aren't officially being worked on any more but should be relatively stable.

And also regarding the last idea of bumping the previous version to 0.6.0 and then making anything before 1.0.0 be for Vue 2 I don't actually have a big opinion but I think that it could work and wouldn't depart from your existing messaging. My understanding is that, technically, the current version is already supporting Vue 3 only anyway.

For example: https://github.com/morkro/vue-a11y-dialog/pull/31/files#diff-b335630551682c19a781afebcf4d07bf978fb1f8ac04c6bf87428ed5106870f5R7 just updates what you had before. I suppose that messaging would just switch to 0.6.0. As I prepared the PR I realized Vue 2 support required using a specific branch anyway. So since it's sort of already been established I don't think it's a big deal. Not sure what others feel.

I would also add that, in my opinion, Vue 2 support should only be needed (in terms of the community's needs) for some sort of finite period of time before it's sort of end of lifed; and hopefully the entire community will move to Vue 3 in a year or so. That said, if folks really need Vue 2 support, the branches would still be there for them to use.

For the Vue 2 version. Bug fixes probably wouldn't make it into that branch unless absolutely egregious. I don't think folks would have time for that. But I believe it's fairly stable anyway, correct?


I updated my PR which I've placed on hold with a small checklist of items like linting and generated /dist verification. Of course if you get a chance to take a look at the overall code to see if there's anything else that'd be great. Otherwise, I think most of what's left is pretty straight-forward. I just need to find spare cycles to get it done.

Regarding vue-demi, it's an interesting idea! I have not really looked at or used that before. My opinion is that we should just embrace Vue 3 + Vite and the composition api and move forward and leave the Vue 2 version available for those that are not ready to make the switch; and honestly, not put too much effort into accommodating legacy versions by adding dependencies. In my experience, the added complexity and effort to accommodate backwards compatibility in this sort of way should be questioned and weighed against comparable effort to move forward and fully support latest Vue 3 et al idioms. This is especially true for OSS projects where folks are struggling to find time to work on projects in their spare cycles.

Furthermore, I thought to have read that Vue 2.7 will be the last minor release for v2, but can't find the information anymore.

Right! So it's sort of like IE11 EOL June 15 in a way...let's fully embrace all the "new ways" and push the web forward 🍰 💥 💯 🚀

@roblevintennis
Copy link
Contributor

@morkro I've finished my checklist and here's the verification notes:
#31 (comment)

So I took it out of draft mode and it is now ready for review.

@morkro
Copy link
Owner

morkro commented Jan 25, 2022

I've just published 1.1.0 with the upgrade to a11y-dialog@7.3.0 🎉 It's available on npm already as well.

Huge thanks to @roblevintennis for all your work on the upgrade and continuous push to get this released :)

@roblevintennis
Copy link
Contributor

Awesome! 🎉

Thanks @morkro and @KittyGiraudel for allowing me to actively contribute and get involved with this project overall — it was fun contributing and collaborating to get this done and of course it feels especially great to contribute to a proven accessible and inclusive dialog component like a11y-dialog.

@morkro morkro closed this as completed Jan 25, 2022
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

No branches or pull requests

5 participants