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

Added primary changes to add page for Removed icons #159

Closed
wants to merge 5 commits into from

Conversation

jorgeamadosoria
Copy link
Contributor

@jorgeamadosoria jorgeamadosoria commented Dec 12, 2021

Added Removed icons page, to contain all deleted icons in previous version with a format similar to the one at the main page. Some icons that were removed because the brand does not allow modifications are therefore listed, but not shown in compliance with the reason they were removed (i.e., Fedora, Yandex, etc).

Pending changes: navigation to and fro the main page.

Alternatively, we can just forgo navigation and link this from the Readme in the main repository, as a note.

Copy link
Member

@LitoMore LitoMore left a comment

Choose a reason for hiding this comment

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

.DS_Store should not be committed.

@sachinraja
Copy link
Contributor

Just a small tip - you should add it to your global gitignore

.gitignore Outdated Show resolved Hide resolved
@@ -0,0 +1,314 @@
module.exports = [
Copy link
Member

Choose a reason for hiding this comment

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

We need a way to update this file automatically. Do you have something in mind about it @jorgeamadosoria?

Copy link
Contributor Author

@jorgeamadosoria jorgeamadosoria Dec 25, 2021

Choose a reason for hiding this comment

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

No. I don't really mind updating this manually. We don't remove that many icons, this is easy to keep updated just by reading the Release notifications and getting the removed issues from there.
I'm fine with this setup.
Of course, if you can think of a way to automate it, I'm all ears. The less work, the better.

Copy link
Member

@mondeja mondeja Dec 25, 2021

Choose a reason for hiding this comment

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

I think that it should be inside the "Auto Release" workflow, after the step that bumps simple-icons version.

Technically would be "if there is a major version change, compare the icons in latest release with the new ones, and add those deleted to the file".

No. I don't really mind updating this manually.

If so, you maybe need to create a release checklist and declare some compromise to maintain it. Anyways, please, note that we don't want to depend on a single contributor for periodical manual tasks. If you left the organization tomorrow for whatever reason someone else would be obliged to keep this, so I am totally against not automating it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, no I get that. Not sure how to add this to the Auto Release workflow. I'll take a look.

Comment on lines +1 to +27
doctype html
html(lang='en-US')
head
meta(charset='UTF-8')
meta(
content='initial-scale=1, shrink-to-fit=no, width=device-width',
name='viewport'
)
link(rel='preconnect', href='https://fonts.gstatic.com')
title #{ pageTitle }
meta(name='description', content=pageDescription)
meta(property='og:type', content='website')
meta(property='og:title', content=pageTitle)
meta(property='og:description', content=pageDescription)
meta(property='og:url', content=pageUrl)
meta(property='og:site_name', content=pageTitle)
meta(property='og:image', content=`${pageUrl}/images/og.png`)
link(rel='icon', type='image/x-icon', href=`${pageUrl}/images/favicon.ico`)
link(rel='icon', type='image/png', href=`${pageUrl}/images/favicon.png`)
link(rel='apple-touch-icon', href=`${pageUrl}/images/apple-touch-icon.png`)
link(rel='mask-icon', href=`${pageUrl}/images/logo.svg`, color='#111111')
link(
rel='stylesheet',
href='https://fonts.googleapis.com/css2?family=Open+Sans:wght@600&family=Roboto+Mono:wght@400;600&display=swap'
)
link(rel='license', href='license.txt')
link(rel='canonical', href=pageUrl)
Copy link
Member

Choose a reason for hiding this comment

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

The head and the footer should be reused between the two pages. See Template Inheritance with Pug.

Comment on lines +144 to +169
new HtmlWebpackPlugin({
inject: true,
filename: 'removed.html',
template: path.resolve(ROOT_DIR, 'removed.pug'),
templateParameters: {
removedIcons: removedIcons
.sort((v1, v2) => v1.title.localeCompare(v2.title))
.map((icon, iconIndex) => {
const luminance = getRelativeLuminance(`#${icon.hex}`);
return {
base64Svg: Buffer.from(icon.path).toString('base64'),
hex: icon.hex,
light: luminance < 0.4,
superLight: luminance > 0.95,
superDark: luminance < 0.02,
issue: icon.issue,
versionIssue: icon.versionIssue,
path: icon.path,
title: icon.title,
};
}),
iconCount: removedIcons.length,
pageTitle: 'Removed Icons',
pageDescription:
"Removed icons in all previous versions. Some icons can't be shown since they were removed because the brand does not allow them to be modified to fit this library.",
pageUrl: 'https://simpleicons.org/removed',
Copy link
Member

Choose a reason for hiding this comment

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

A lot of configuration can also be reused between the two pages, this should help us maintaining the project.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

agreed.

@mondeja mondeja added the enhancement New feature or request label Dec 23, 2021
@jorgeamadosoria
Copy link
Contributor Author

The changes necessary to automate the removed icons page are not worth the trouble. I'm closing this in favor of greater focus on the other open PRs on the website.

@jorgeamadosoria jorgeamadosoria added the abandoned Pull requests that have been abandoned by the contributor label Dec 30, 2021
@LitoMore LitoMore deleted the issue-148 branch December 30, 2021 05:41
@LitoMore LitoMore restored the issue-148 branch December 30, 2021 05:42
@adamrusted adamrusted deleted the issue-148 branch August 29, 2023 11:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
abandoned Pull requests that have been abandoned by the contributor enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add "museum" page for discontinued brands
4 participants