-
-
Notifications
You must be signed in to change notification settings - Fork 278
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
Feature/sc 30090/confirm rtl changes still work in new sheets #2201
Merged
yitzhakc
merged 225 commits into
modularization-main
from
feature/sc-30090/confirm-rtl-changes-still-work-in-new-sheets
Jan 6, 2025
Merged
Feature/sc 30090/confirm rtl changes still work in new sheets #2201
yitzhakc
merged 225 commits into
modularization-main
from
feature/sc-30090/confirm-rtl-changes-still-work-in-new-sheets
Jan 6, 2025
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…g them to fit the expected old api response.
… to include an object for any language (he and en) rather than a string. The idea is to enable it to include other details and not only versionTitle.
…mbers and use it rather than placeSegmentNuncers.
…itle should be displayed.
…n there is more than one source.
…n the aources attribute is in the response.
…o components (rather than get it from cache objects).
…ng the translation VersionTitle.
…ned object in ssr processing.
…as languageFamilyName. this enables finding versions when we have only versionTitle without languageFamilyName (this happens in search results).
…s API accurately - with he/en meaning primary/translation and with the languageFamilyName param.
…with useEffect. control closing when clicking inside menu by adding a classname for preventing closing
…o dicta results..
feat(elasticSearch): add languageFamilyName and isPrimary to search API result.
…for segment level.
…HigherPriority in reduce call, for higher readability.
…utButton out of LayoutButtons.
yitzhakc
requested changes
Dec 24, 2024
…s for clicking on the open/close button and for click inside the menu contents.
…till-work-in-new-sheets
yitzhakc
approved these changes
Jan 6, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Merged rtl-client into modularization-main and made changes to DropdownMenu.jsx and s2.css so that both types of dropdown menu work.
Code Changes
Change name of props of buttonContent and menuIconComponent to buttonComponent both in and in all instances of it
Change .buttonClosest into a data attribute data-prevent-close="true"
CSS resolutions
dropdownButton needs to be resolved to dropdownLinks-button (note that it was changed from a button html element to an a)
Note that in rtl-client {children} is directly inside div.dropdownLinks-menu whereas in modularization-main it's enclosed within an additional div.dropdownLinks-options
We've removed the additional enclosing div from modularization main
No default export when importing DropdownMenu.jsx
An additional enclosing div (or component?) should be added to the instances of in the Header.jsx
The instances of in rtl-client are all enclosed with divs that set the positioning style
The CSS styling used in .header .dropdownLinks should be migrated to a class that the newly created enclosing div/component (used in Header.jsx) will be tagged with
The CSS styling on should be as sparse as possible
Add an additional prop to called positioningClass that will be added to the list of class -es in the "first"
I.e. in rtl instances use dropdownMenu and in Header.jsx use dropdownLinks -- CHANGE ALL THESE CLASS NAMES (readerDropdownMenu vs headerDropdownMenu)
Now that we've removed the
in rtl instances this is already taken care of