Skip to content

PB-1680 : Fix icon selector border#1350

Merged
sami-nouidri-swisstopo merged 1 commit intodevelopfrom
bug-PB-1680-icon-selector-bug
May 22, 2025
Merged

PB-1680 : Fix icon selector border#1350
sami-nouidri-swisstopo merged 1 commit intodevelopfrom
bug-PB-1680-icon-selector-bug

Conversation

@sami-nouidri-swisstopo
Copy link
Contributor

@sami-nouidri-swisstopo sami-nouidri-swisstopo commented May 21, 2025

Also changed the border class to be tailwind-compliant

Here's the result :
image

Test link

@cypress
Copy link

cypress bot commented May 21, 2025

web-mapviewer    Run #5373

Run Properties:  status check passed Passed #5373  •  git commit a8fe2c923e: PB-1680 : Fix icon selector border
Project web-mapviewer
Branch Review bug-PB-1680-icon-selector-bug
Run status status check passed Passed #5373
Run duration 06m 13s
Commit git commit a8fe2c923e: PB-1680 : Fix icon selector border
Committer Sami Nouidri
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 0
Tests that did not run due to a developer annotating a test with .skip  Pending 20
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 252
View all changes introduced in this branch ↗︎

<div
v-if="currentIconSet && currentIconSet.icons.length > 0"
class="icon-selector border-2 bg-light rounded"
class="icon-selector bg-light rounded border border-gray-300"
Copy link
Contributor

Choose a reason for hiding this comment

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

wait... border-gray-300 that's some Tailwind???
how the hell is this not prefixed by tw:? I'm confused

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah, but we import Tailwind with @import 'tailwindcss' prefix(tw) important;, so it should be tw:border-gray-300, so why is it still interpreted?
That's a huge deal, as the class border is the same in both library, so if we now know that Tailwind doesn't care about our prefix, and still applies its classes without the prefix, we should move to Tailwind ASAP instead of keeping Bootstrap alongside it

@sami-nouidri-swisstopo sami-nouidri-swisstopo force-pushed the bug-PB-1680-icon-selector-bug branch from 8631eb5 to a8fe2c9 Compare May 22, 2025 09:00
@sami-nouidri-swisstopo sami-nouidri-swisstopo merged commit 3a179f7 into develop May 22, 2025
6 checks passed
@sami-nouidri-swisstopo sami-nouidri-swisstopo deleted the bug-PB-1680-icon-selector-bug branch May 22, 2025 09:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants