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

Create a Wikibase discovery page #719

Merged
merged 22 commits into from
Sep 11, 2023
Merged

Create a Wikibase discovery page #719

merged 22 commits into from
Sep 11, 2023

Conversation

AndrewKostka
Copy link
Contributor

No description provided.

@github-actions
Copy link

Deployment previews on netlify for branch refs/pull/719/merge will be at the following locations (when build is done):

@AndrewKostka AndrewKostka marked this pull request as ready for review September 6, 2023 09:24
const pseudorandom = {
seed: 1,
next: function () {
const x = Math.sin(this.seed++) * 10000
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice, quite a satisfying way to do this

},
color: function () {
const colors = ['red', 'blue', 'green', 'purple']
return this.logo ? 'white' : colors[Math.floor(Math.random() * colors.length)]
Copy link
Contributor

Choose a reason for hiding this comment

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

Not really an issue but just a note: this means on recreating the component the colour will change


<style scoped>
.grid {
display: grid;
Copy link
Contributor

Choose a reason for hiding this comment

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

This is cool, I didn't know this was a thing (masonry grids in native css) until I saw this

}
},
methods: {
resizeCards () {
Copy link
Contributor

Choose a reason for hiding this comment

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

This is totally fine, no need to change but I was wondering if there was a more native vuetify way to do this (e.g. maybe using it's breakpoint functionality)

const start = (currentPage - 1) * resultsPerPage
const end = start + resultsPerPage

return {
Copy link
Contributor

Choose a reason for hiding this comment

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

Just generally I love this super comprehensive mock data; makes testing the thing really convenient

@@ -1,8 +1,8 @@
<template>
<v-app id="app">
<Navbar></Navbar>

<v-container class="full-height-content">
<router-view v-if="customLayout"/>
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess this might be a sign in future we should move the "rows and cols" into each page. I suspect in most cases each "page" really wants control of it's layout rather than having it imposed on it at the App level

Copy link
Contributor

@tarrow tarrow left a comment

Choose a reason for hiding this comment

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

Looks great; thanks for such a comprehensive patch; was interesting to read and I certainly now have some more stuff to read up on about grid layouts

@AndrewKostka AndrewKostka merged commit 6dcae41 into main Sep 11, 2023
7 checks passed
@AndrewKostka AndrewKostka deleted the discovery-feature branch September 11, 2023 12:22
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