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 styling for dead services that indicate that they're no longer available #3139

Open
wants to merge 46 commits into
base: main
Choose a base branch
from

Conversation

cmho
Copy link
Contributor

@cmho cmho commented Jun 23, 2023

CODE TOUR: I went on an arduous journey and learned a lot about arrays in Perl to tweak username display so that dead services aren't linked out and display a default userhead.

Closes #3056.

Copy link
Member

@kareila kareila left a comment

Choose a reason for hiding this comment

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

Thanks so much for working on this! I hope you are enjoying the process 😅

Copy link
Member

@kareila kareila left a comment

Choose a reason for hiding this comment

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

I feel like this is getting close! Adding [site] to the printed username is something requested from the original issue that I just noticed was missing; the rest of my comments are mere syntax suggestions. 👍

@@ -69,13 +70,17 @@ sub ljuser_display {
my $display_class = $opts{no_ljuser_class} ? "" : " class='ljuser'";
my $domain = $self->site->{domain} ? $self->site->{domain} : $self->site->{hostname};

$nolink = 1 if exists $DW::External::Site::deadsites{$domain};
Copy link
Member

Choose a reason for hiding this comment

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

I would suggest combining this line and the initial nolink declaration as follows:

my $nolink = exists $DW::External::Site::deadsites{$domain} ? 1 : 0;

This is called the ternary operator 😄

Copy link
Member

Choose a reason for hiding this comment

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

Since we're now using a get_deadsites function, this syntax needs to be updated:

my %deadsites = DW::External::Site::get_deadsites();
my $nolink = exists $deadsites{$domain} ? 1 : 0;

Sorry for not noticing that in my previous review! I think that's the last update necessary for this to work.

I would also highly encourage you to make sure your Perl edits are tidy going forward. @momijizukamori posted a pre-commit hook in dw-dev recently to help make this easier!

@cmho cmho requested a review from kareila July 26, 2023 22:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

Add "this service is defunct" styling to old user name tags
3 participants