Skip to content

fix: swap components over to use the composition api#6

Open
CaramelKat wants to merge 11 commits into
masterfrom
feat/composition-api
Open

fix: swap components over to use the composition api#6
CaramelKat wants to merge 11 commits into
masterfrom
feat/composition-api

Conversation

@CaramelKat
Copy link
Copy Markdown
Member

Resolves #XXX

Changes:

Per feedback, updated components to use the composition api instead of the older method for defining props

Copy link
Copy Markdown
Member

@binaryoverload binaryoverload left a comment

Choose a reason for hiding this comment

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

For the theming, instead of setting classes using a query selector across the whole app - I think you should look at using a state management library like https://pinia.vuejs.org/

This will allow you to get/set the theme anywhere in the app - and then set classes on elements appropriately

Comment thread app/components/editTag.vue Outdated
Comment thread src/app/components/editTag.vue Outdated
Comment thread src/app/components/editTag.vue Outdated
Comment thread src/app/components/playingCard.vue Outdated
Comment thread app/components/timeline.vue
Comment thread src/app/components/timeline.vue Outdated
Comment on lines +11 to +14
if(!document.fonts.check('10px "PixelMplus12-Regular"'))
window.setTimeout(loadText, 100);
else
window.matrix.setText(pages);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is this needed? If we put loadText into an onMount hook then it will always be loaded

This will also cause an infinite loop if the font can't be loaded for some reason

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This is needed because for whatever reason, the font doesn't always get loaded at the right time, which often results in the following:

onMounted(() => {
	matrix.setText(pages);
});
image

Comment thread src/app/components/userSettings.vue Outdated
Comment thread app/components/userSettings.vue Outdated
Copy link
Copy Markdown
Member

@binaryoverload binaryoverload left a comment

Choose a reason for hiding this comment

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

Meant for the previous review to be request changes oops

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

Successfully merging this pull request may close these issues.

2 participants