Skip to content

Bugfix: Add the correct techs to the compare button when loading a page#1025

Merged
tunetheweb merged 2 commits intomainfrom
cwvtech-bugfix-compare
Apr 14, 2025
Merged

Bugfix: Add the correct techs to the compare button when loading a page#1025
tunetheweb merged 2 commits intomainfrom
cwvtech-bugfix-compare

Conversation

@sarahfossheim
Copy link
Copy Markdown
Collaborator

Previous behavior in the categories page: on pageload, regardless if any technologies were selected from the URL or not, the "compare technologies" button above the table compared all technologies on the page. Additionally, the URL it led to was formatted wrong (one long string with %2C as a separator instead of commas), and as a result the compare page didn't work.

Updated code:

  • Checks first if techs are selected or not
  • If any techs are selected, those are used
  • If not, all techs on the page are used
  • Then the list is separated by comma before being encoded

Result:

  • Enter the page with no techs selected → Link leads to: Comparison page with all techs in the table
  • Enter the page with one or more techs selected → Link leads to: Comparison page with the selected techs
  • The actual techs are loaded in the comparison page

Copy link
Copy Markdown
Member

@tunetheweb tunetheweb left a comment

Choose a reason for hiding this comment

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

The changes work...

However I notice the compare pages still uses %2C when selecting items (/reports/techreport/category?geo=ALL&rank=ALL&category=CMS&selected=Shopify%2CWordPress)

The link is right, but since we use a comma for the techreport URLs (/reports/techreport/tech?tech=Shopify,WordPress)

Similarly manually selecting techs in the techreport URL and clicking update uses %2C: /reports/techreport/tech?tech=WordPress%2CShopify&geo=ALL&rank=ALL

I wonder if we use consistently use a comma in all URLs for consistency?

@sarahfossheim
Copy link
Copy Markdown
Collaborator Author

yes, will fix that, looks nicer when it's consistent!

Copy link
Copy Markdown
Member

@tunetheweb tunetheweb left a comment

Choose a reason for hiding this comment

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

LGTM thanks!

@tunetheweb tunetheweb merged commit 7457717 into main Apr 14, 2025
8 checks passed
@tunetheweb tunetheweb deleted the cwvtech-bugfix-compare branch April 14, 2025 17:46
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