Draft
Conversation
jonbarrow
requested changes
Feb 20, 2024
Member
jonbarrow
left a comment
jonbarrow
left a comment
There was a problem hiding this comment.
Mostly just style and consistency changes. Most of this will be redone/removed in the future so I'm not too worried about the finer details right now. Once the admin panel is done being rewritten I planned to add a kanban board to it and ditch GitHub Projects entirely, so once that's done we can do all this tagging and filtering and stuff in a more streamlined way
public/assets/js/filter-progress.js
Outdated
Comment on lines
+1
to
+24
| const progressCards = document.getElementsByClassName('purple-card') | ||
|
|
||
| function showWithFilter(filter){ | ||
| for (const card of progressCards){ | ||
| const type = card.getAttribute('type') | ||
| if (filter === "none" || type.toLowerCase().includes(filter)){ | ||
| card.classList.remove('off') | ||
| } | ||
| else { | ||
| card.classList.add('off') | ||
| } | ||
| } | ||
| } | ||
|
|
||
| const buttons = document.getElementById('filter-container').children | ||
| let selectedButton = buttons[0] | ||
| for (const button of buttons){ | ||
| button.addEventListener('click',(event) => { | ||
| selectedButton.classList.remove('primary') | ||
| selectedButton = event.target | ||
| selectedButton.classList.add('primary') | ||
| showWithFilter(event.target.id); | ||
| }) | ||
| } No newline at end of file |
Member
There was a problem hiding this comment.
Suggested change
| const progressCards = document.getElementsByClassName('purple-card') | |
| function showWithFilter(filter){ | |
| for (const card of progressCards){ | |
| const type = card.getAttribute('type') | |
| if (filter === "none" || type.toLowerCase().includes(filter)){ | |
| card.classList.remove('off') | |
| } | |
| else { | |
| card.classList.add('off') | |
| } | |
| } | |
| } | |
| const buttons = document.getElementById('filter-container').children | |
| let selectedButton = buttons[0] | |
| for (const button of buttons){ | |
| button.addEventListener('click',(event) => { | |
| selectedButton.classList.remove('primary') | |
| selectedButton = event.target | |
| selectedButton.classList.add('primary') | |
| showWithFilter(event.target.id); | |
| }) | |
| } | |
| const progressCards = document.getElementsByClassName('purple-card'); | |
| function showWithFilter(filter) { | |
| for (const card of progressCards) { | |
| const type = card.getAttribute('type'); | |
| if (filter === "none" || type.toLowerCase().includes(filter)) { | |
| card.classList.remove('off'); | |
| } else { | |
| card.classList.add('off'); | |
| } | |
| } | |
| } | |
| const buttons = document.getElementById('filter-container').children; | |
| let selectedButton = buttons[0]; | |
| for (const button of buttons) { | |
| button.addEventListener('click', (event) => { | |
| selectedButton.classList.remove('primary'); | |
| selectedButton = event.target; | |
| selectedButton.classList.add('primary'); | |
| showWithFilter(event.target.id); | |
| }) | |
| } |
Member
There was a problem hiding this comment.
I can't make multiple suggestions over the same lines in one review, but I would also prefer to not use the children property on filter-container. Instead using querySelectorAll targeting the buttons would be safer in the event that we need to restructure the HTML and not worry about updating the JS
src/cache.js
Outdated
| } | ||
| } | ||
| } | ||
| `; //Loophole to get the ReadMe No Matter the branch name |
Member
There was a problem hiding this comment.
The website is behind on this, but we've been moving to the comment format made by better comments
Suggested change
| `; //Loophole to get the ReadMe No Matter the branch name | |
| `; // * Loophole to get the ReadMe No Matter the branch name |
src/cache.js
Outdated
Comment on lines
+76
to
+85
| main: object(expression: "main:README.md") { | ||
| ... on Blob { | ||
| text | ||
| } | ||
| } | ||
| master: object(expression: "master:README.md") { | ||
| ... on Blob { | ||
| text | ||
| } | ||
| } |
Member
There was a problem hiding this comment.
You don't have to do this in 2 steps. Target HEAD rather than a branch and it gives you the repositories default branch
Suggested change
| main: object(expression: "main:README.md") { | |
| ... on Blob { | |
| text | |
| } | |
| } | |
| master: object(expression: "master:README.md") { | |
| ... on Blob { | |
| text | |
| } | |
| } | |
| readme: object(expression: "HEAD:README.md") { | |
| ... on Blob { | |
| text | |
| } | |
| } |
src/cache.js
Outdated
| } | ||
| `; //Loophole to get the ReadMe No Matter the branch name | ||
|
|
||
| const serviceApps = ["account","nex","friends","wii u chat", "juxt", "website"] |
Member
There was a problem hiding this comment.
Suggested change
| const serviceApps = ["account","nex","friends","wii u chat", "juxt", "website"] | |
| const serviceApps = ["account", "nex", "friends", "wii u chat", "juxt", "website"]; |
This could also probably go on multiple lines, but I won't require it
src/cache.js
Outdated
Comment on lines
+155
to
+216
| async function getRepoType(name, title){ | ||
| const data = await github.request(GetRepositoryDescription, { | ||
| orgName: 'PretendoNetwork', | ||
| repoName: name | ||
| }) | ||
| let readMe = "" | ||
| if (data.repository.main != null) | ||
| readMe = data.repository.main.text | ||
| else | ||
| readMe = data.repository.master.text | ||
|
|
||
| readMe = readMe.split('\n')[0].toLowerCase() | ||
| let description = data.repository.description | ||
| if (description != null) | ||
| description = description.toLowerCase() | ||
| else | ||
| description = "" | ||
| return await setRepoType(title.toLowerCase(),description,readMe) | ||
| } | ||
|
|
||
|
|
||
| async function setRepoType(title, description, readMe){ | ||
| let types = [] | ||
| let isGame = true | ||
| for (let app of serviceApps) | ||
| if (title.includes(app)){ | ||
| types.push("Service") | ||
| isGame = false | ||
| break | ||
| } | ||
|
|
||
| if (isGame) { | ||
| types.push("Game") | ||
| } | ||
|
|
||
| if (title.includes('(') && isGame){ | ||
| if (title.includes('3ds')) | ||
| types.push("3DS") | ||
| else | ||
| types.push("Wii U") | ||
| return types | ||
| } | ||
|
|
||
|
|
||
| if (title === 'nex' || title.includes('juxt') || title.includes('account')){ | ||
| types.push('3DS') | ||
| types.push('Wii U') | ||
| if (title.includes('juxt')) | ||
| types.push("Website") | ||
| } | ||
| else if (title.includes('web')){ | ||
| types.push("Website") | ||
| } | ||
| else if (description.includes('3ds') || readMe.includes('3ds') || title.includes('3ds')){ | ||
| types.push('3DS') | ||
| } | ||
| else{ | ||
| types.push('Wii U') | ||
| } | ||
|
|
||
| return types | ||
| } |
Member
There was a problem hiding this comment.
Suggested change
| async function getRepoType(name, title){ | |
| const data = await github.request(GetRepositoryDescription, { | |
| orgName: 'PretendoNetwork', | |
| repoName: name | |
| }) | |
| let readMe = "" | |
| if (data.repository.main != null) | |
| readMe = data.repository.main.text | |
| else | |
| readMe = data.repository.master.text | |
| readMe = readMe.split('\n')[0].toLowerCase() | |
| let description = data.repository.description | |
| if (description != null) | |
| description = description.toLowerCase() | |
| else | |
| description = "" | |
| return await setRepoType(title.toLowerCase(),description,readMe) | |
| } | |
| async function setRepoType(title, description, readMe){ | |
| let types = [] | |
| let isGame = true | |
| for (let app of serviceApps) | |
| if (title.includes(app)){ | |
| types.push("Service") | |
| isGame = false | |
| break | |
| } | |
| if (isGame) { | |
| types.push("Game") | |
| } | |
| if (title.includes('(') && isGame){ | |
| if (title.includes('3ds')) | |
| types.push("3DS") | |
| else | |
| types.push("Wii U") | |
| return types | |
| } | |
| if (title === 'nex' || title.includes('juxt') || title.includes('account')){ | |
| types.push('3DS') | |
| types.push('Wii U') | |
| if (title.includes('juxt')) | |
| types.push("Website") | |
| } | |
| else if (title.includes('web')){ | |
| types.push("Website") | |
| } | |
| else if (description.includes('3ds') || readMe.includes('3ds') || title.includes('3ds')){ | |
| types.push('3DS') | |
| } | |
| else{ | |
| types.push('Wii U') | |
| } | |
| return types | |
| } | |
| async function getRepoType(name, title) { | |
| const data = await github.request(getRepositoryDescription, { | |
| orgName: 'PretendoNetwork', | |
| repoName: name | |
| }); | |
| const readme = data.repository.readme.text.split('\n')[0].toLowerCase(); | |
| const description = data.repository.description?.toLowerCase() || ''; | |
| return setRepoType(title.toLowerCase(), description, readme); | |
| } | |
| function setRepoType(title, description, readMe) { | |
| const types = []; | |
| let isGame = true; | |
| for (const app of serviceApps) { | |
| if (title.includes(app)) { | |
| types.push('Service'); | |
| isGame = false; | |
| break; | |
| } | |
| } | |
| if (isGame) { | |
| types.push('Game'); | |
| } | |
| if (title.includes('(') && isGame) { | |
| if (title.includes('3ds')) { | |
| types.push('3DS'); | |
| } else { | |
| types.push('Wii U'); | |
| } | |
| return types; | |
| } | |
| if (title === 'nex' || title.includes('juxt') || title.includes('account')) { | |
| types.push('3DS'); | |
| types.push('Wii U'); | |
| if (title.includes('juxt')) { | |
| types.push('Website'); | |
| } | |
| } else if (title.includes('web')) { | |
| types.push('Website'); | |
| } else if (description.includes('3ds') || readMe.includes('3ds') || title.includes('3ds')) { | |
| types.push('3DS'); | |
| } else { | |
| types.push('Wii U'); | |
| } | |
| return types; | |
| } |
src/cache.js
Outdated
Comment on lines
+72
to
+73
| const GetRepositoryDescription = gql` | ||
| query GetRepositoryDescription($orgName: String!, $repoName: String!){ |
Member
There was a problem hiding this comment.
Suggested change
| const GetRepositoryDescription = gql` | |
| query GetRepositoryDescription($orgName: String!, $repoName: String!){ | |
| const getRepositoryDescription = gql` | |
| query getRepositoryDescription($orgName: String!, $repoName: String!){ |
src/cache.js
Outdated
| todo: [] | ||
| } | ||
| }, | ||
| type: [] |
Member
There was a problem hiding this comment.
Suggested change
| type: [] | |
| types: [] |
src/cache.js
Outdated
Comment on lines
+267
to
+268
| const types = await getRepoType(project.url.split("/")[4], project.title) | ||
| extractedData.type = types |
Member
There was a problem hiding this comment.
Suggested change
| const types = await getRepoType(project.url.split("/")[4], project.title) | |
| extractedData.type = types | |
| extractedData.types = await getRepoType(project.url.split("/")[4], project.title); |
Or just set this directly on the object rather than reassigning it here
Need Mobile layout design
|
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR (which I will trun into a draft) adds the filter Progress like in the Figma Design
The Filters then can also be localised if needed
Old:
New: