Deprecate slots, attributes and syntax by itsyme · Pull Request #2208 · MarkBind/markbind

itsyme · 2023-03-13T05:45:54Z

What is the purpose of this pull request?

Overview of changes:
Resolves #1854. Fully deprecated Modal: modal-header, modal-footer, Question: hasInput, invalid question types (invalid type attribute), Icons: {{ }} syntax.

modal-header and modal-footer in Modal, hasInput and invalid question types (invalid type attribute) in Question, {{ }} syntax in icons have been fully deprecated

To migrate:
Modal:

modal-header -> header
- modal-header attribute has been fully deprecated
- To migrate, replace the modal-header attribute with just header with the text in the attribute unchanged
- e.g. <modal modal-header="HEADER TEXT"> to <modal header="HEADER TEXT">
modal-footer -> footer
- modal-footer attribute has been fully deprecated
- To migrate, replace the modal-footer attribute with just footer with the text in the attribute unchanged
- e.g. <modal modal-footer="FOOTER TEXT"> to <modal footer="FOOTER TEXT">

Question:

has-input -> type="text"
- has-input attribute has been fully deprecated
- To migrate, replace the has-input attribute with type="text"
- e.g. <question has-input> to <question type="text">
Invalid question types (invalid type attribute) now not supported
- To migrate, give the question a valid type: mcq, checkbox, blanks, text

Icons:

{{ }} -> : :
- To migrate, replace all {{ and }} to :
- e.g. {{emoji-name}} to :emoji-name:

Anything you'd like to highlight/discuss:

Testing instructions:

Proposed commit message: (wrap lines at 72 characters)
Deprecate slots, attributes and syntax

Modal: modal-header, modal-footer, Question: hasInput,
invalid question types (questions without input fields), Icons: {{ }}
syntax are supported.

This could be confusing for users as it is not standardised and some
slots, attributes or syntax are not mentioned in documentation.

Let's fully deprecate these slots, attributes and syntax fully and
remove them from documentation.

This prevents confusion for users by standardising the slots,
attributes and syntax to use, which is made clear in documentation.

Checklist: ☑️

Updated the documentation for feature additions and enhancements
Added tests for bug fixes or features
Linked all related issues
No unrelated changes

jovyntls

Thanks @itsyme !
Would you mind looking into deprecating these as well? If they're related to the current deprecation issue they can be deprecated; if they are not related, we can document any possible changes in (user-facing) behaviour and update the migration information

jovyntls · 2023-03-14T13:50:14Z

packages/core/src/html/warnings.ts

-      'modal-header': 'header',
-      'modal-footer': 'footer',
-    });
+    _warnDeprecatedSlotNames(node, {});


We can remove this method entirely since we'll no longer have deprecated slot names 🎉

jovyntls

Thanks for the deprecation work @itsyme :)

I notice there's a <question> without any type in the test_site - can we add a question type to make sure we have no unexpected/undefined behaviour in our tests?

jovyntls · 2023-03-21T13:18:37Z

packages/vue-components/src/questions/Question.vue

+    isNotTextWithoutKeywords() {
+      return !(this.isTextQuestion() && !this.keywords);


This method name is quite confusing with a double negation (and then a triple negation above when we are negating it to be assigned to const isTextWithoutKeyword above) 😅 Can we rename this? Maybe isTextWithoutKeyword where it can be used directly?

jovyntls · 2023-03-21T13:25:41Z

packages/core/src/html/warnings.ts

-    });
-  },
-};
+export const warnDeprecatedAtributesMap: { [attr: string]: (nd: MbNode) => void } = {};


Same for this - let's remove it entirely

jovyntls · 2023-03-21T13:26:01Z

packages/core/src/html/warnings.ts

+// eslint-disable-next-line @typescript-eslint/no-unused-vars
 function _warnDeprecatedSlotNames(element: MbNode, namePairs: { [name: string]: string }) {


We should be able to remove this function entirely as well! 🎉
If we need it again (hopefully never), we can recover it from the history :)

jovyntls · 2023-03-21T13:27:13Z

packages/core/test/unit/html/NodeProcessor.data.ts

-`;
-
-export const PROCESS_MODAL_SLOTS_RENAMING_EXPECTED = `
+module.exports.PROCESS_MODAL_SLOTS_RENAMING_EXPECTED = `


This should be removed as well - it's the expected output of the removed test case

jovyntls

LGTM! One last thing, can we update the issue description to be more informative to anyone who might be migrating?

To migrate:
Modal: modal-header -> header, modal-footer -> footer
Question: hasInput -> type="text", Invalid question types (invalid type attribute) now not supported
Icons: {{ }} -> : :

e.g. details could include:

what modal-header is (slot/attribute/etc)
what users can do if they are currently using an invalid question type that is no longer supported
any other helpful information

Ideally we should be able to put it in the release notes to help users migrating from v4 to v5 :)

itsyme · 2023-03-23T06:09:20Z

LGTM! One last thing, can we update the issue description to be more informative to anyone who might be migrating?

To migrate:
Modal: modal-header -> header, modal-footer -> footer
Question: hasInput -> type="text", Invalid question types (invalid type attribute) now not supported
Icons: {{ }} -> : :

e.g. details could include:

what modal-header is (slot/attribute/etc)

what users can do if they are currently using an invalid question type that is no longer supported

any other helpful information

Ideally we should be able to put it in the release notes to help users migrating from v4 to v5 :)

Hi @jovyntls! I've edited the description! Do let me know if it is alright or if there is anything that could be improved!

jovyntls

LGTM! It's great that we're deprecating these, thanks @itsyme 😀

jovyntls · 2023-03-23T06:45:27Z

@itsyme Would you like to merge this PR? Remember to use the squash-merge strategy and to add the proposed commit message in the description :)

Feel free to merge it after a second approval has been given, or after 24 hours (following the DG project management guidelines)

itsyme added 10 commits March 11, 2023 18:05

Remove icon deprecation from docs

3701700

Deprecate has-input in docs

5db171a

Deprecate modal-footer from docs

740c541

Remove warnings modal-footer, modal-header

65d2c79

Deprecate modal-footer, modal-header fully

348be42

Question: Deprecate hasInput

868d2c3

Question: Deprecate isValidQuestionType

7ec04fe

Update tests

bf27e31

Merge branch 'master' into deprecate-1845

3d99318

Remove modal-header from unit tests

84cd3f4

jovyntls reviewed Mar 14, 2023

View reviewed changes

itsyme added 8 commits March 14, 2023 22:23

Remove warnDeprecatedSlotNames call

b689527

Merge master

0c70c2e

Merge master

a4d32b6

Reinstate icons

d63acbd

Question: Deprecate test and guard clause

2b58df0

Merge master

a4f7a7a

Resolve conflict

e5e748e

Remove deprecate comment

caa1752

itsyme requested a review from jovyntls March 21, 2023 09:56

jovyntls suggested changes Mar 21, 2023

View reviewed changes

itsyme added 3 commits March 21, 2023 23:03

Fix nits

d41d427

Update tests

bb22655

Merge branch 'master' into deprecate-1845

2e964cd

jovyntls reviewed Mar 22, 2023

View reviewed changes

jovyntls added this to the v4.1.1 milestone Mar 23, 2023

jovyntls approved these changes Mar 23, 2023

View reviewed changes

jovyntls added the breakingChange 💥 Feature will behave significantly different, or is made obsolete label Mar 23, 2023

jovyntls removed this from the v4.1.1 milestone Mar 23, 2023

jovyntls added this to the v5.0.0 milestone Mar 23, 2023

Merge branch 'master' into deprecate-1845

c8e1a88

itsyme merged commit a422eb5 into MarkBind:master Mar 25, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Deprecate slots, attributes and syntax#2208

Deprecate slots, attributes and syntax#2208
itsyme merged 22 commits intoMarkBind:masterfrom
itsyme:deprecate-1845

itsyme commented Mar 13, 2023 •

edited

Loading

Uh oh!

jovyntls left a comment

Uh oh!

jovyntls Mar 14, 2023

Uh oh!

jovyntls left a comment

Uh oh!

jovyntls Mar 21, 2023

Uh oh!

jovyntls Mar 21, 2023

Uh oh!

jovyntls Mar 21, 2023

Uh oh!

jovyntls Mar 21, 2023

Uh oh!

jovyntls left a comment •

edited

Loading

Uh oh!

itsyme commented Mar 23, 2023

Uh oh!

jovyntls left a comment

Uh oh!

jovyntls commented Mar 23, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		isNotTextWithoutKeywords() {
		return !(this.isTextQuestion() && !this.keywords);

		// eslint-disable-next-line @typescript-eslint/no-unused-vars
		function _warnDeprecatedSlotNames(element: MbNode, namePairs: { [name: string]: string }) {

Conversation

itsyme commented Mar 13, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jovyntls left a comment

Choose a reason for hiding this comment

Uh oh!

jovyntls Mar 14, 2023

Choose a reason for hiding this comment

Uh oh!

jovyntls left a comment

Choose a reason for hiding this comment

Uh oh!

jovyntls Mar 21, 2023

Choose a reason for hiding this comment

Uh oh!

jovyntls Mar 21, 2023

Choose a reason for hiding this comment

Uh oh!

jovyntls Mar 21, 2023

Choose a reason for hiding this comment

Uh oh!

jovyntls Mar 21, 2023

Choose a reason for hiding this comment

Uh oh!

jovyntls left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

itsyme commented Mar 23, 2023

Uh oh!

jovyntls left a comment

Choose a reason for hiding this comment

Uh oh!

jovyntls commented Mar 23, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

itsyme commented Mar 13, 2023 •

edited

Loading

jovyntls left a comment •

edited

Loading