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

Basic rtl #997

Merged
merged 2 commits into from
Jul 31, 2023
Merged

Basic rtl #997

merged 2 commits into from
Jul 31, 2023

Conversation

solomax
Copy link
Contributor

@solomax solomax commented Jul 30, 2023

No description provided.

@solomax solomax mentioned this pull request Jul 30, 2023
@reckart
Copy link
Contributor

reckart commented Jul 30, 2023

It seems that the PR contains a separate version of the compiled bootstrap for RTL languages. Is that observation correct?

@solomax
Copy link
Contributor Author

solomax commented Jul 30, 2023

@reckart yes, bootstrap should be updated before merge (webjar release is not available her)

@reckart
Copy link
Contributor

reckart commented Jul 30, 2023

By updated you mean removed? Why are would separate fully compiled Bootstrap CSS files for RTL and non-RTL be required? Looking at the Bootstrap documentation, the main change towards RTL support in BS5 was to replace the XXX-left/XXX-right classes with XXX-start and XXX-end(e.g. ml-1 -> ms-1). I don't see indications that vanilla BS has any type of build-time switch for RTL-related properties.

@reckart
Copy link
Contributor

reckart commented Jul 30, 2023

Ah, ok... now I see. The included bootstrap.rtl.css is actually part of the upstream distro. I wasn't aware of that:

https://getbootstrap.com/docs/5.3/getting-started/contents/#css-files

@solomax
Copy link
Contributor Author

solomax commented Jul 30, 2023

@reckart was just about to paste same link :))

@reckart
Copy link
Contributor

reckart commented Jul 30, 2023

@solomax Actually, The BS 5.3.1 webjar is available on Maven Central: https://search.maven.org/artifact/org.webjars.npm/bootstrap/5.3.1/jar

Unzipping the file, I can also see the RTL variations of the CSS files in there.

@solomax
Copy link
Contributor Author

solomax commented Jul 30, 2023

Thanks!
I'll update this pr with new bootstrap a bit later tonight :)

https://mvnrepository.com/artifact/org.webjars/bootstrap displays outdated info :(

@solomax
Copy link
Contributor Author

solomax commented Jul 30, 2023

Maven coordinates are wrong :(

we are using org.webjars:bootstrap while at your link the artifact is org.webjars.npm:bootstrap

I've created an issue: webjars/bootstrap#184

@reckart
Copy link
Contributor

reckart commented Jul 30, 2023

Any reason not to use the org.webjars.npm version?

According to https://www.webjars.org, the org.webjars.npm version mirrors the contents of the respective NPM package whereas org.webjars must be manually built and deployed - seems unnecessary overhead?

@reckart
Copy link
Contributor

reckart commented Jul 30, 2023

Also, if a new version of an org.webjars.npm package is required, it appears this can be done directly from the webjars website using the Add webjar... button, just by entering the package name and selecting the version desired.

@solomax
Copy link
Contributor Author

solomax commented Jul 30, 2023

Any reason not to use the org.webjars.npm version?

These jars have different structure
Maybe resource load will miss in case org.webjars.npm:bootstrap will be used

Anyway this question is for another PR :)))

@martin-g martin-g merged commit c1fe914 into martin-g:wicket-9.x-bootstrap-5.x Jul 31, 2023
@martin-g
Copy link
Owner

Thank you, @solomax !

@solomax solomax deleted the basic-rtl branch July 31, 2023 13:43
@solomax
Copy link
Contributor Author

solomax commented Jul 31, 2023

@martin-g could you please cherry-pick this one to wicket-10 branch? :)

@martin-g
Copy link
Owner

Done!

@solomax
Copy link
Contributor Author

solomax commented Aug 1, 2023

@martin-g for whatever reason I don't see this commit here: https://github.com/l0rdn1kk0n/wicket-bootstrap/commits/wicket-10.x-bootstrap-5.x :(((

martin-g pushed a commit that referenced this pull request Aug 1, 2023
* RTL for vanilla bootstrap is added

* RTL support for bootswatch was added

(cherry picked from commit c1fe914)
@martin-g
Copy link
Owner

martin-g commented Aug 1, 2023

It was my fault. Just pushed it.

@solomax
Copy link
Contributor Author

solomax commented Aug 1, 2023

Thanks! :)))

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.

3 participants