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

Bootstrap 5 #1934

Merged
merged 43 commits into from
Feb 14, 2024
Merged

Bootstrap 5 #1934

merged 43 commits into from
Feb 14, 2024

Conversation

jakobvogel
Copy link
Member

@jakobvogel jakobvogel commented Feb 9, 2024

Description

Ports Tycho to Bootstrap 5.

Additional Notes

Checklist

  • Code change has been tested and works locally
  • Code was formatted via IntelliJ and follows SonarLint & best practices

In general, Bootstrap 5 replaced `left` with `start` and `right` with `end`, in order to support Right-to-Left languages. Also includes further adaptions. See https://getbootstrap.com/docs/5.0/migration/.

SIRI-925
The name has used twice. Charts could no longer be closed.

SIRI-933
Introduced margin that is no longer implied, added compulsory groups to labels.

SIRI-925
Wrapping appendices to inputs is no longer required in BS 5.

SIRI-925
Links are underlined, jQuery removed.

SIRI-925
@jakobvogel jakobvogel added 🖐 Keep open Should not be merged 💣 BREAKING CHANGE Contains non-backwards compatible changes to public methods or changes the behavior of existing code 🕔 Wait for sirius Needs new version from another sirius lib to work labels Feb 9, 2024
Lists must not contain links directly, but they must reside within list items.

SIRI-925
The component makes assumptions about the inner workings of Bootstrap that are no longer met by version 5. It turns out that changing the tabs via the drop-down menu (as happens if there are more than 4 languages) does not work initially, due to a conflicting inner state between ourselves and bootstrap. For now, this can be solved by simulating a click on the entry for the selected language. This will bring the states into synch.

SIRI-925
@sabieber
Copy link
Member

sabieber commented Feb 13, 2024

I see this script only used in tycho-biz.js.pasta, which is not used in the S2 FE.

Thanks for checking 👍🏻 Did not check it myself, just saw that the file is in a generic scripts package and not in a tycho package.

This commit uses a new key introduced in scireum/sirius-web@c58dec1.

SIRI-925
The file is never directly loaded but invoked via another template.

SIRI-925
As proposed by @sabieber, we can reuse the link element instead of manually replicating the link syntax multiple times.

SIRI-925
@jakobvogel jakobvogel removed the 🖐 Keep open Should not be merged label Feb 13, 2024
@jakobvogel jakobvogel removed the 🕔 Wait for sirius Needs new version from another sirius lib to work label Feb 14, 2024
@jakobvogel jakobvogel marked this pull request as ready for review February 14, 2024 08:56
@jakobvogel jakobvogel merged commit c744e0b into develop Feb 14, 2024
2 checks passed
@jakobvogel jakobvogel deleted the feature/jvo/SIRI-925-Bootstrap-5 branch February 14, 2024 08:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
💣 BREAKING CHANGE Contains non-backwards compatible changes to public methods or changes the behavior of existing code 🤯 Complex PR with extensive changes that should be reviewed with extra care ⬆️ Dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants