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

Fix several b-modal selector names #1218

Merged
merged 1 commit into from
May 4, 2020

Conversation

ang-zeyu
Copy link
Contributor

@ang-zeyu ang-zeyu commented May 2, 2020

What is the purpose of this pull request? (put "X" next to an item, remove the rest)

• [x] Bug fix

Fixes #1217

What changes did you make? (Give an overview)
Change leftover 'modal' selectors from the bootstrap vue migration to 'b-modal'

Testing instructions:

  • npm run test should pass

Proposed commit message: (wrap lines at 72 characters)
Fix several b-modal selector names

The bootstrap vue migration causes modal elements to be transformed into
b-modal.

Let’s change the leftover selector names to match the new name.

The bootstrap vue migration causes modal elements to be transformed into
b-modal.

Let’s change the leftover selector names to match the new name.
@ang-zeyu ang-zeyu requested a review from nbriannl May 2, 2020 09:03
@nbriannl
Copy link
Contributor

nbriannl commented May 4, 2020

Not very familiar, so I need to make some clarifications before I approve. Basically, #1033 migrated modals to b-modals, but there were some parts of the code, namely the files you edited, that still used the previous name? So you finished up the leftovers?

@ang-zeyu
Copy link
Contributor Author

ang-zeyu commented May 4, 2020

it was missed out in #1033, and only surfaced now. Could double check with @openorclose if there's a reason it wasn't changed though (unlikely)

Copy link
Contributor

@nbriannl nbriannl left a comment

Choose a reason for hiding this comment

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

it was missed out in #1033, and only surfaced now. Could double check with @openorclose if there's a reason it wasn't changed though (unlikely)

Should be unlikely, LGTM

@openorclose
Copy link
Contributor

it was missed out in #1033, and only surfaced now. Could double check with @openorclose if there's a reason it wasn't changed though (unlikely)

No reason, I missed it out. Thanks for the fix!

@openorclose openorclose closed this May 4, 2020
@openorclose openorclose reopened this May 4, 2020
@ang-zeyu
Copy link
Contributor Author

ang-zeyu commented May 4, 2020

ok will merge it in then, thanks for looking through it @nbriannl @openorclose!

@ang-zeyu ang-zeyu added this to the v2.15.0 milestone May 4, 2020
@ang-zeyu ang-zeyu added the pr.BugFix 🐛 Fixes correct a programming error/assumption label May 4, 2020
@ang-zeyu ang-zeyu merged commit a515669 into MarkBind:master May 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr.BugFix 🐛 Fixes correct a programming error/assumption
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Some pageNav links don't work, possibly due to the use of colon
3 participants