Use 'string' library to generate panel header's id by nusjzx · Pull Request #89 · MarkBind/vue-strap

nusjzx · 2018-07-30T04:12:28Z

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

• [ ] Documentation update
• [X] Bug fix
• [ ] New feature
• [ ] Enhancement to an existing feature
• [ ] Other, please explain:

Fixes MarkBind/markbind#392.

What is the rationale for this request?
The MarkBind repo is using markdown-it-anchor to generate the ids for Markdown heading, and it uses the string library. For consistency, we should use the same library for vue-strap.

What changes did you make? (Give an overview)
Import the string library, use it to generate the ids instead of using our own regex.

Testing instructions:

npm install
run npm run build in vue-strap repo
copy vue-strap.min.js to markbind repo
test on 2103 website.
eg: search "unsafe"

yamgent · 2018-07-30T06:18:14Z

As discussed, fix doesn't work with MarkBind/markbind#392 (comment).

yamgent · 2018-07-30T08:00:15Z

src/Panel.vue

+        const panelHeaderText = jQuery(this.headerContent).wrap('<div></div>').parent().find(':header').text();
        this.$refs.cardContainer.setAttribute('id', string(panelHeaderText).slugify().toString());
+      } else if(this.$refs.headerWrapper.innerHTML) {
+        this.$refs.cardContainer.setAttribute('id', jQuery(this.$refs.headerWrapper.innerHTML).wrap('<div></div>').parent().find(':header')[0].id);


Will this cause an error if there is no header at all?

Maybe try something like this instead?

const header = this.$refs.headerWrapper.innerHTML).wrap('<div></div>').parent().find(':header'); if (header.length > 0) { this.$refs.cardContainer.setAttribute('id', header[0].id); }

yamgent · 2018-07-30T08:00:45Z

src/Panel.vue

+
+      if (this.headerContent) {
+        const panelHeaderText = jQuery(this.headerContent).wrap('<div></div>').parent().find(':header').text();
        this.$refs.cardContainer.setAttribute('id', string(panelHeaderText).slugify().toString());


you will still need to check if (panelHeaderText) {, otherwise it is possible for the id to be blank.

yamgent

As above. If you have made the changes, do remember to push them to your branch.

yamgent · 2018-07-31T03:05:24Z

src/Panel.vue

+          this.$refs.cardContainer.setAttribute('id', string(panelHeaderText).slugify().toString());
+        }
+      } else if(this.$refs.headerWrapper.innerHTML) {
+        const header = this.$refs.headerWrapper.innerHTML).wrap('<div></div>').parent().find(':header');


This line doesn't compile (it should be jQuery(this.$refs.headerWrapper.innerHTML)?)

yamgent · 2018-07-31T03:06:50Z

src/Panel.vue

+
+      if (this.headerContent) {
+        const panelHeaderText = jQuery(this.headerContent).wrap('<div></div>').parent().find(':header').text();
+        if(panelHeaderText) {


Should have a space after if: if (panelHeaderText) {

yamgent · 2018-07-31T03:07:22Z

src/Panel.vue

+        if(panelHeaderText) {
+          this.$refs.cardContainer.setAttribute('id', string(panelHeaderText).slugify().toString());
+        }
+      } else if(this.$refs.headerWrapper.innerHTML) {


Same issue: should have space after if.

yamgent

Works fine now. You can squash the commits now.

yamgent · 2018-07-31T04:11:34Z

Not sure what happened, but I can't get it to work now? It now shows this error on the console:

TypeError: Cannot read property 'innerHTML' of undefined
    at o.mounted (vue-strap.min.js:37)
    at _t (vue.min.js:6)
    at Object.insert (vue.min.js:6)
    at A (vue.min.js:6)
    at hn.__patch__ (vue.min.js:6)
    at hn._update (vue.min.js:6)
    at hn.<anonymous> (vue.min.js:6)
    at St.get (vue.min.js:6)
    at new St (vue.min.js:6)
    at hn.$mount (vue.min.js:6)

Also the commit message can be a bit better (can just be the same as the PR title).

yamgent

I still cannot jump to tutorial-timetable-tutor-roletimesvenue. I checked the source and apparently the ID is not generated?

yamgent · 2018-07-31T05:42:16Z

src/Panel.vue

+        if (panelHeaderText) {
+          this.$refs.cardContainer.setAttribute('id', string(panelHeaderText).slugify().toString());
+        }
+      } else if (this.$refs.innerHTML) {


Is this supposed to be this.$refs.headerWrapper.innerHTML?

yamgent changed the title ~~Import string module~~ Use 'string' module to generate panel header's id Jul 30, 2018

yamgent changed the title ~~Use 'string' module to generate panel header's id~~ Use 'string' library to generate panel header's id Jul 30, 2018

yamgent approved these changes Jul 30, 2018

View reviewed changes

yamgent modified the milestone: v2.0.1-markbind.17 Jul 30, 2018

yamgent reviewed Jul 30, 2018

View reviewed changes

yamgent modified the milestone: v2.0.1-markbind.17 Jul 30, 2018

yamgent suggested changes Jul 31, 2018

View reviewed changes

yamgent reviewed Jul 31, 2018

View reviewed changes

nusjzx force-pushed the import-string-module branch from 140593b to 8b5d16f Compare July 31, 2018 03:35

nusjzx force-pushed the import-string-module branch from dc5458f to 25b946e Compare July 31, 2018 05:03

yamgent suggested changes Jul 31, 2018

View reviewed changes

yamgent reviewed Jul 31, 2018

View reviewed changes

Use 'string' library to generate panel header's id

cad5f67

nusjzx force-pushed the import-string-module branch from 25b946e to cad5f67 Compare July 31, 2018 05:49

yamgent approved these changes Jul 31, 2018

View reviewed changes

yamgent added this to the v2.0.1-markbind.17 milestone Jul 31, 2018

yamgent merged commit 2af0d7f into MarkBind:master Jul 31, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use 'string' library to generate panel header's id#89

Use 'string' library to generate panel header's id#89
yamgent merged 1 commit intoMarkBind:masterfrom
nusjzx:import-string-module

nusjzx commented Jul 30, 2018 •

edited by yamgent

Loading

Uh oh!

yamgent commented Jul 30, 2018

Uh oh!

yamgent Jul 30, 2018

Uh oh!

yamgent Jul 30, 2018

Uh oh!

yamgent left a comment

Uh oh!

yamgent Jul 31, 2018

Uh oh!

yamgent Jul 31, 2018

Uh oh!

yamgent Jul 31, 2018

Uh oh!

yamgent left a comment •

edited

Loading

Uh oh!

yamgent commented Jul 31, 2018 •

edited

Loading

Uh oh!

yamgent left a comment

Uh oh!

yamgent Jul 31, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

nusjzx commented Jul 30, 2018 • edited by yamgent Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

yamgent commented Jul 30, 2018

Uh oh!

yamgent Jul 30, 2018

Choose a reason for hiding this comment

Uh oh!

yamgent Jul 30, 2018

Choose a reason for hiding this comment

Uh oh!

yamgent left a comment

Choose a reason for hiding this comment

Uh oh!

yamgent Jul 31, 2018

Choose a reason for hiding this comment

Uh oh!

yamgent Jul 31, 2018

Choose a reason for hiding this comment

Uh oh!

yamgent Jul 31, 2018

Choose a reason for hiding this comment

Uh oh!

yamgent 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!

yamgent commented Jul 31, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

yamgent left a comment

Choose a reason for hiding this comment

Uh oh!

yamgent Jul 31, 2018

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

nusjzx commented Jul 30, 2018 •

edited by yamgent

Loading

yamgent left a comment •

edited

Loading

yamgent commented Jul 31, 2018 •

edited

Loading