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

Fixed issue # Fixed issue #18639: Token browse can be unusable with lot of data in attribute #3082

Merged
merged 3 commits into from May 2, 2023

Conversation

Shnoulle
Copy link
Collaborator

Dev: longtext formatter + scss
Dev: TODO css

…ot of data in attribute

Dev: longtext formatter + scss
Dev: TODO css
@Shnoulle
Copy link
Collaborator Author

@ptelu : how to construct the css via the scss ?

Or where ad this scss fix ?

Dev: Add grid-view-ls to class of all admin gris
@Shnoulle
Copy link
Collaborator Author

Thank you,

Seems grid-view-ls was removed between 5.X to 6.X, i add it again.

OK or need another class ?

@sonarcloud
Copy link

sonarcloud bot commented Apr 26, 2023

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@Shnoulle
Copy link
Collaborator Author

@ptelu just to know if there are a reason to remove grid-view-ls from yiigrid ?

I add it again ? Maybe need to create a new class and fix from scratch ?

@ptelu
Copy link
Contributor

ptelu commented Apr 27, 2023

i think its fine yes, we removed it mostly because everything was based on the table class. But i agree that gridviews should have a distinct name. We didn't get to moving the remnants remnants out of adminbasics_temporary. They have their own component folder in the theme "assets/admin_themes/Sea_Green/tables". I would rename/readd the grid class to "ls-gridview" then it would fit with our custom naming as well as with the controller.

For this fix i think it is fine if you add the class again, we can add another task to rename/readd and move the code to the right folder.

@tiborpacalat
Copy link
Collaborator

I have tested this and it works now as expected.
Screenshot 2023-04-27 at 18 00 22

@tiborpacalat tiborpacalat added Tested OK This PR has been tested by QA and works as expected and removed Needs testing labels Apr 27, 2023
@Shnoulle
Copy link
Collaborator Author

See othe scss part : https://github.com/LimeSurvey/LimeSurvey/blob/master/assets/admin_themes/adminbasics_temporary/grid.scss

Some seems great , unsure on some other

@Shnoulle
Copy link
Collaborator Author

I have tested this and it works now as expected.

Did you see : seems BS5 have fixed the issue with tooltip too.

@Shnoulle Shnoulle merged commit 77a234a into master May 2, 2023
18 of 23 checks passed
@Shnoulle Shnoulle deleted the bug/18639_6X_tokenBrowse branch May 2, 2023 13:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs code review Tested OK This PR has been tested by QA and works as expected
Projects
None yet
3 participants