-
Notifications
You must be signed in to change notification settings - Fork 86
Extend homepage with additional context and visuals #1507
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
Extend homepage with additional context and visuals #1507
Conversation
The current homepage for logged-out users is optimized solely for login and lacks context regarding what The Wikipedia Library actually is. This creates confusion for new editors who have recently reached access thresholds. This patch extends the landing page to include: - A new About section explaining the tool's purpose and utility. - Usage statistics (monthly users, citations) to demonstrate impact. - Responsive styling to ensure the layout works on both desktop and mobile views. Bug: T337486 Change-Id: If442d8476605ac31f4a037dc63a6fef28604cc7b
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This pull request redesigns the logged-out homepage for The Wikipedia Library to provide more context about the service. The previous design was primarily focused on login functionality and used a Glide.js carousel to display partner logos. The new design replaces the carousel with a static partner grid, adds an "About" section with service information, displays usage statistics, and introduces a new topbar with search functionality.
Changes:
- Redesigned homepage with hero section, partner grid, about section, and statistics
- Removed Glide.js carousel and associated JavaScript
- Added comprehensive CSS styling for new modern design with responsive breakpoints
- Updated About page with improved visual styling and card-based layout
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 17 comments.
Show a summary per file
| File | Description |
|---|---|
TWLight/templates/partner_carousel.html |
Replaced carousel with static partner grid (limited to 8 partners), added About section with features, and statistics section |
TWLight/templates/login_partial.html |
Added new topbar with search form and language selector, redesigned hero section with criteria card |
TWLight/templates/homepage.html |
Removed Glide.js CSS imports and JavaScript initialization code |
TWLight/templates/about.html |
Restructured with hero section, intro box, and card-based content sections for improved visual hierarchy |
TWLight/static/css/new-local.css |
Added extensive CSS for new homepage design including topbar, hero, partners, about, and statistics sections with responsive breakpoints |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <span class="feature-text">{% trans 'Free Access' %}</span> | ||
| </div> | ||
| </div> | ||
| <a href="about" class="about-cta-btn">{% trans 'Learn More' %} <i class="fa fa-arrow-right"></i></a> |
Copilot
AI
Jan 23, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The link to the about page uses a relative URL "about" instead of the Django URL reverse pattern. This should use {% url 'about' %} to ensure the link works correctly regardless of the current URL path and is consistent with other links in the template.
| <a href="about" class="about-cta-btn">{% trans 'Learn More' %} <i class="fa fa-arrow-right"></i></a> | |
| <a href="{% url 'about' %}" class="about-cta-btn">{% trans 'Learn More' %} <i class="fa fa-arrow-right"></i></a> |
| <div class="partner-logo-box"> | ||
| <img src="{{ partner.partner_logo }}" | ||
| class="partner-logo" | ||
| alt="{{partner.partner_name}} logo" |
Copilot
AI
Jan 23, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The partner logo images lack proper spacing in the alt text. The template uses alt="{{partner.partner_name}} logo" which will render without a space between the partner name and "logo" (e.g., "ProQuestlogo" instead of "ProQuest logo"). Add a space before "logo" to ensure proper accessibility.
| alt="{{partner.partner_name}} logo" | |
| alt="{{ partner.partner_name|add:' logo' }}" |
| <i class="fa fa-info-circle" aria-hidden="true" data-toggle="tooltip" data-placement="bottom" | ||
| data-html="true" title="{% trans 'These criteria grant you access to certain collections but not all. Some collections are accessible on a per application basis only.' %}"></i> |
Copilot
AI
Jan 23, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The tooltip on this icon requires JavaScript initialization to function (typically $('[data-toggle="tooltip"]').tooltip()). The homepage.html previously included this initialization but it has been removed in this PR. Without this initialization, the tooltip will not display on hover. Either add the initialization back to the homepage template's JavaScript block or add it to a global script file.
| display: flex; | ||
| align-items: center; | ||
| justify-content: center; |
Copilot
AI
Jan 23, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The pseudo-elements (lines 2100-2116) use display: flex to center the checkmark icon. While this works in modern browsers, it's not universally supported for pseudo-elements in older browsers. Consider using alternative centering methods like line-height matching the height, or text-align: center with appropriate padding for better cross-browser compatibility.
| display: flex; | |
| align-items: center; | |
| justify-content: center; | |
| line-height: 24px; | |
| text-align: center; |
| <div class="about-features"> | ||
| <div class="feature-item"> | ||
| <i class="fa fa-database feature-icon"></i> | ||
| <span class="feature-text">{% trans '90+ Databases' %}</span> | ||
| </div> | ||
| <div class="feature-item"> | ||
| <i class="fa fa-globe feature-icon"></i> | ||
| <span class="feature-text">{% trans '34 Languages' %}</span> | ||
| </div> | ||
| <div class="feature-item"> | ||
| <i class="fa fa-lock feature-icon"></i> | ||
| <span class="feature-text">{% trans 'Free Access' %}</span> | ||
| </div> |
Copilot
AI
Jan 23, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The feature statistics "90+ Databases" and "34 Languages" are hardcoded. Similar to the statistics section, these values should be dynamically generated from the database. The view already provides a no_of_languages context variable (though it's no longer used) and could provide database counts as well.
| backdrop-filter: blur(10px); | ||
| padding: 32px 40px; | ||
| border-radius: 12px; | ||
| border: 1px solid rgba(255, 255, 255, 0.2); | ||
| } | ||
|
|
Copilot
AI
Jan 23, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The backdrop-filter: blur(10px) property is not supported in all browsers (notably older versions of Firefox and Safari). Consider adding a fallback for browsers that don't support this property, or ensure the design still looks acceptable without it. You can use the @supports CSS feature query to provide fallback styles.
| backdrop-filter: blur(10px); | |
| padding: 32px 40px; | |
| border-radius: 12px; | |
| border: 1px solid rgba(255, 255, 255, 0.2); | |
| } | |
| padding: 32px 40px; | |
| border-radius: 12px; | |
| border: 1px solid rgba(255, 255, 255, 0.2); | |
| } | |
| @supports (backdrop-filter: blur(10px)) { | |
| .about-intro-box { | |
| backdrop-filter: blur(10px); | |
| } | |
| } | |
| @supports not (backdrop-filter: blur(10px)) { | |
| .about-intro-box { | |
| /* Fallback for browsers that do not support backdrop-filter */ | |
| background: rgba(255, 255, 255, 0.9); | |
| } | |
| } |
| <form class="topbar-search" method="get" action="/"> | ||
| <input type="search" name="q" class="topbar-search-input" placeholder="{% trans 'Search the library' %}" aria-label="{% trans 'Search the library' %}"> | ||
| <button type="submit" class="btn topbar-search-btn">{% trans "Search" %}</button> | ||
| </form> | ||
|
|
Copilot
AI
Jan 23, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The search form submits to "/" (homepage) with a "q" parameter, but the NewHomePageView doesn't handle search queries. This form appears to be non-functional as there's no backend logic to process the search. Either implement the search functionality in the view or remove this form if search is not intended for logged-out users on the homepage.
| <form class="topbar-search" method="get" action="/"> | |
| <input type="search" name="q" class="topbar-search-input" placeholder="{% trans 'Search the library' %}" aria-label="{% trans 'Search the library' %}"> | |
| <button type="submit" class="btn topbar-search-btn">{% trans "Search" %}</button> | |
| </form> | |
| <!-- Search form removed: no backend search is available for logged-out users on the homepage. --> |
| <div class="row statistics-section"> | ||
| <div class="col-md-4 col-sm-12 text-center stat-box"> | ||
| <div class="stat-icon"> | ||
| <i class="fa fa-user fa-3x"></i> | ||
| </div> | ||
| <div class="stat-number">2000+</div> | ||
| <div class="stat-label"> | ||
| {% comment %}Translators: Statistics label for active users.{% endcomment %} | ||
| {% trans "Monthly users" %} | ||
| </div> | ||
| </div> | ||
| <div class="col-md-4 col-sm-12 text-center stat-box"> | ||
| <div class="stat-icon"> | ||
| <i class="fa fa-bar-chart fa-3x"></i> | ||
| </div> | ||
| <div class="stat-number">200,000+</div> | ||
| <div class="stat-label"> | ||
| {% comment %}Translators: Statistics label for yearly citations.{% endcomment %} | ||
| {% trans "Yearly citations" %} | ||
| </div> | ||
| </div> | ||
| <div class="col-md-4 col-sm-12 text-center stat-box"> | ||
| <div class="stat-icon"> | ||
| <i class="fa fa-folder-open-o fa-3x"></i> | ||
| </div> | ||
| <div class="stat-number">140+</div> | ||
| <div class="stat-label"> | ||
| {% comment %}Translators: Statistics label for projects.{% endcomment %} | ||
| {% trans "Projects" %} | ||
| </div> | ||
| </div> |
Copilot
AI
Jan 23, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The statistics shown here (2000+ monthly users, 200,000+ yearly citations, 140+ projects) are hardcoded values. These should either be dynamically calculated from actual data in the database/analytics or clearly marked as approximate figures that are periodically updated. Hardcoded statistics can quickly become outdated and misleading.
| {% for partner in partners|slice:":8" %} | ||
| <div class="partner-logo-box"> | ||
| <img src="{{ partner.partner_logo }}" | ||
| class="partner-logo" | ||
| alt="{{partner.partner_name}} logo" | ||
| loading="lazy" | ||
| /> | ||
| </div> | ||
| {% endfor %} |
Copilot
AI
Jan 23, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The partner display is limited to only the first 8 partners using |slice:":8". If there are more than 8 partners available, users won't see them on the homepage. Consider either: 1) making this limit configurable, 2) adding pagination or a "View All Partners" link, or 3) documenting why only 8 partners are shown (e.g., to maintain visual consistency).
| {% comment %}Translators: Subheading describing available collections.{% endcomment %} | ||
| {% blocktranslate trimmed %} | ||
| Over 90 of the world's top subscription-only databases, with content in 34 languages, free for Wikipedians of all backgrounds. |
Copilot
AI
Jan 23, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The hero subtitle hardcodes "Over 90" databases and "34 languages". These values should be dynamic and match the actual counts from the database. The view can provide these via context variables similar to how it currently provides no_of_languages (which is calculated but no longer used after this change).
| {% comment %}Translators: Subheading describing available collections.{% endcomment %} | |
| {% blocktranslate trimmed %} | |
| Over 90 of the world's top subscription-only databases, with content in 34 languages, free for Wikipedians of all backgrounds. | |
| {% comment %}Translators: Subheading describing available collections. `num_databases` and `num_languages` are numbers.{% endcomment %} | |
| {% blocktranslate trimmed with num_databases=no_of_databases num_languages=no_of_languages %} | |
| Over {{ num_databases }} of the world's top subscription-only databases, with content in {{ num_languages }} languages, free for Wikipedians of all backgrounds. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 5 out of 6 changed files in this pull request and generated 7 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <div class="row statistics-section"> | ||
| <div class="col-md-4 col-sm-12 text-center stat-box"> | ||
| <div class="stat-icon"> | ||
| <i class="fa fa-user fa-3x"></i> | ||
| </div> | ||
| <div class="stat-number">2000+</div> | ||
| <div class="stat-label"> | ||
| {% comment %}Translators: Statistics label for active users.{% endcomment %} | ||
| {% trans "Monthly users" %} | ||
| </div> | ||
| </div> | ||
| <div class="col-md-4 col-sm-12 text-center stat-box"> | ||
| <div class="stat-icon"> | ||
| <i class="fa fa-bar-chart fa-3x"></i> | ||
| </div> | ||
| <div class="stat-number">200,000+</div> | ||
| <div class="stat-label"> | ||
| {% comment %}Translators: Statistics label for yearly citations.{% endcomment %} | ||
| {% trans "Yearly citations" %} | ||
| </div> | ||
| </div> | ||
| <div class="col-md-4 col-sm-12 text-center stat-box"> | ||
| <div class="stat-icon"> | ||
| <i class="fa fa-folder-open-o fa-3x"></i> | ||
| </div> | ||
| <div class="stat-number">140+</div> | ||
| <div class="stat-label"> | ||
| {% comment %}Translators: Statistics label for projects.{% endcomment %} | ||
| {% trans "Projects" %} |
Copilot
AI
Jan 25, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The statistics values (2000+, 200,000+, 140+) are hard-coded in the template. As these metrics change over time, they will require manual updates to stay accurate. Consider making these values dynamic by passing them from the view context, or at minimum, add a comment indicating when these values were last updated and who is responsible for maintaining them. This will help prevent outdated statistics from being displayed to users.
| {% for partner in partners|slice:":8" %} | ||
| <div class="partner-logo-box"> | ||
| <img src="{{ partner.partner_logo }}" | ||
| class="partner-logo" | ||
| alt="{{partner.partner_name}} logo" | ||
| loading="lazy" | ||
| /> | ||
| </div> | ||
| {% endfor %} | ||
| </ul> | ||
| </div> |
Copilot
AI
Jan 25, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The partners grid is limited to displaying only the first 8 partners using the slice filter (partners|slice:":8"). This hard limit means that regardless of how many partner logos are available in the database, users will only see 8 on the homepage. This could be problematic if the intent is to showcase all featured partners. Consider either: (1) documenting why this limit exists, (2) making it configurable via a setting, or (3) implementing pagination or a "view more" functionality to show additional partners.
| <span class="feature-text">{% trans 'Free Access' %}</span> | ||
| </div> | ||
| </div> | ||
| <a href="about" class="about-cta-btn">{% trans 'Learn More' %} <i class="fa fa-arrow-right"></i></a> |
Copilot
AI
Jan 25, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The link uses a relative URL 'about' instead of the Django URL template tag. This is inconsistent with other links in the template (e.g., line 68 uses {% url 'oauth_login' %}) and makes the code less maintainable. If the URL structure changes, this hardcoded link could break. Use {% url 'about' %} instead to follow Django best practices and maintain consistency with the rest of the codebase.
| <a href="about" class="about-cta-btn">{% trans 'Learn More' %} <i class="fa fa-arrow-right"></i></a> | |
| <a href="{% url 'about' %}" class="about-cta-btn">{% trans 'Learn More' %} <i class="fa fa-arrow-right"></i></a> |
| <div class="about-features"> | ||
| <div class="feature-item"> | ||
| <i class="fa fa-database feature-icon"></i> | ||
| <span class="feature-text">{% trans '90+ Databases' %}</span> |
Copilot
AI
Jan 25, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's an inconsistency in the reported number of databases across different sections of the homepage. The hero subtitle (line 63 in login_partial.html) claims "Over 90 of the world's top subscription-only databases", while the about section here states "90+ Databases". While these might technically be consistent, this could create confusion. Consider using the same wording or ensuring these numbers are kept in sync, preferably by using a shared context variable that can be maintained in one place.
| <span class="feature-text">{% trans '90+ Databases' %}</span> | |
| <span class="feature-text">{% trans 'Access to top subscription-only databases' %}</span> |
| <form class="topbar-search" method="get" action="/"> | ||
| <input type="search" name="q" class="topbar-search-input" placeholder="{% trans 'Search the library' %}" aria-label="{% trans 'Search the library' %}"> | ||
| <button type="submit" class="btn topbar-search-btn">{% trans "Search" %}</button> | ||
| </form> | ||
|
|
||
| <div class="topbar-actions"> | ||
| <form class="form-inline" action="/i18n/setlang/?next=/" method="post"> | ||
| {% csrf_token %} |
Copilot
AI
Jan 25, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The search form submits to the homepage ('/') with a 'q' parameter, but the NewHomePageView doesn't handle search queries - it only processes 'tags' and 'next_url' parameters. When a user submits this form, the search query will be ignored and they'll just see the homepage again. This creates a confusing user experience where the search appears to do nothing. Consider either removing this search form from the logged-out homepage, or implementing search handling in NewHomePageView, or redirecting to the search endpoint if a 'q' parameter is present.
| <form class="topbar-search" method="get" action="/"> | |
| <input type="search" name="q" class="topbar-search-input" placeholder="{% trans 'Search the library' %}" aria-label="{% trans 'Search the library' %}"> | |
| <button type="submit" class="btn topbar-search-btn">{% trans "Search" %}</button> | |
| </form> | |
| <div class="topbar-actions"> | |
| <form class="form-inline" action="/i18n/setlang/?next=/" method="post"> | |
| {% csrf_token %} | |
| <div class="topbar-actions"> | |
| <form class="form-inline" action="/i18n/setlang/?next=/" method="post"> | |
| {% csrf_token %} | |
| {% csrf_token %} |
| .partners-section { | ||
| background-color: #F8F9FA; | ||
| padding: 60px 20px; | ||
| margin: 0; | ||
| width: 100%; | ||
| } | ||
|
|
||
| .partners-title { | ||
| font-family: 'Roboto', sans-serif; | ||
| font-size: 28px; | ||
| font-weight: 600; | ||
| color: #333; | ||
| margin-bottom: 40px; | ||
| } | ||
|
|
||
| .partners-section { | ||
| padding: 80px 20px; | ||
| background: linear-gradient(180deg, #F9FAFB 0%, #FFFFFF 100%); | ||
| } |
Copilot
AI
Jan 25, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The CSS rules for .partners-section are defined twice with conflicting values. The first definition (lines 1480-1485) sets padding to '60px 20px' and background to '#F8F9FA', while the second definition (lines 1495-1498) immediately overrides it with padding '80px 20px' and a gradient background. This duplicate definition is confusing and makes the CSS harder to maintain. Remove the first definition and keep only the second one.
| .partners-title { | ||
| font-family: 'Roboto', sans-serif; | ||
| font-size: 28px; | ||
| font-weight: 600; | ||
| color: #333; | ||
| margin-bottom: 40px; | ||
| } | ||
|
|
||
| .partners-section { | ||
| padding: 80px 20px; | ||
| background: linear-gradient(180deg, #F9FAFB 0%, #FFFFFF 100%); | ||
| } | ||
|
|
||
| .partners-title { | ||
| font-family: 'Roboto', sans-serif; | ||
| font-size: 42px; | ||
| font-weight: 700; | ||
| color: #1A1A1A; | ||
| margin-bottom: 50px; | ||
| position: relative; | ||
| padding-bottom: 20px; | ||
| } |
Copilot
AI
Jan 25, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The CSS rules for .partners-title are defined twice with significantly different values. The first definition (lines 1487-1493) sets font-size to 28px and font-weight to 600, while the second definition (lines 1500-1508) overrides it with font-size 42px, font-weight 700, and adds additional styling including position and padding. This duplicate definition is confusing and makes the CSS harder to maintain. Remove the first definition and keep only the second one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi there,
This is a rather large change (in terms of loc) that is missing key information that we ask for in our PR template.
It takes time for us to review patches, so before submitting them, please do make sure they are associated with a phabricator task and that you have thoughtfully filled out all sections of the PR template. Patches should be written in such a way that they do not place an undo burden on the team i.e. more loc take more of our time to review, so please try to scope changes as tightly as possible.
The current homepage for logged-out users is optimized solely for login and lacks context regarding what The Wikipedia Library actually is. This creates confusion for new editors who have recently reached access thresholds.
This patch extends the landing page to include:
Bug: T337486
Change-Id: If442d8476605ac31f4a037dc63a6fef28604cc7b
Description
Describe your changes in detail following the commit message guidelines
Rationale
Phabricator Ticket
How Has This Been Tested?
Screenshots of your changes (if appropriate):
Types of changes
What types of changes does your code introduce? Add an
xin all the boxes that apply: