Skip to content

Conversation

@aalyahya
Copy link
Contributor

Fix: Ensure Unique Tab IDs for Non-Latin Titles

This PR fixes an issue that occurs when using non-Latin titles for tabs without specifying a custom id

Issue:

When using non-Latin titles for tabs without specifying a custom id, the generated HTML assigns duplicate id values due to String#parameterize returning an empty string for non-Latin characters.

Example:

tabs do
  tab "عبدالله" do
    span "tab 1"
  end
  tab "خالد" do
    span "tab 2"
  end
end

Generates:

<nav class="tabs-nav" role="tablist" data-tabs-toggle="#tabs-container-32328">
  <a data-tabs-target="#tabs--32328" role="tab" aria-controls="tabs--32328" href="#" class="text-blue-600 hover:text-blue-600 dark:text-blue-500 dark:hover:text-blue-500 border-blue-600 dark:border-blue-500" aria-selected="true">عبدالله</a>
  <a data-tabs-target="#tabs--32328" role="tab" aria-controls="tabs--32328" href="#" class="dark:border-transparent text-gray-500 hover:text-gray-600 dark:text-gray-400 border-gray-100 hover:border-gray-300 dark:border-gray-700 dark:hover:text-gray-300" aria-selected="false">خالد</a>
</nav>

Since both tabs receive the same id, tab navigation breaks.

Cause

string.parameterize ignores non-Latin characters and returns an empty string:

string = "عبدالله"
string.parameterize #=> ""

Fix:

If parameterize returns an empty string, we now generate a random string using SecureRandom.hex.first(8) to ensure uniqueness. A new instance variable, @fragments, tracks generated IDs to maintain reference consistency.

Changes
• Ensured unique tab id generation when parameterize results in an empty string.
• Introduced @fragments to store generated IDs.
• Added a test case to verify correctness.

…le and not passing custom id

Fix issue in tabs when using non-English letters in tab title
@codecov
Copy link

codecov bot commented Jan 29, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.11%. Comparing base (519e1b9) to head (1b7dcf9).
Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #8616   +/-   ##
=======================================
  Coverage   99.11%   99.11%           
=======================================
  Files         141      141           
  Lines        4073     4074    +1     
=======================================
+ Hits         4037     4038    +1     
  Misses         36       36           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@javierjulio
Copy link
Member

@aalyahya thank you. Sorry, I won't be accepting this change. I've decided for months now that I will remove the tabs component in another v4 beta release as that was my intention. The only component that we don't use directly that will remain is panel. When the tabs component is removed, I will make sure to update the UPGRADING guide to call out that breaking change.

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