Skip to content

Comments

[IMP] openupgrade_160, openupgrade_tools: BS4 to BS5 transformation yeahhhhh#1

Open
daiduongnguyen-odoo wants to merge 21 commits intoViindoo:masterfrom
daiduongnguyen-odoo:master_imp_bs4_to_bs5_migration
Open

[IMP] openupgrade_160, openupgrade_tools: BS4 to BS5 transformation yeahhhhh#1
daiduongnguyen-odoo wants to merge 21 commits intoViindoo:masterfrom
daiduongnguyen-odoo:master_imp_bs4_to_bs5_migration

Conversation

@daiduongnguyen-odoo
Copy link

PR OCA: https://github.com/OCA/openupgradelib/pull/338

@daiduongnguyen-odoo
Copy link
Author

@ngochuy97hp a Huy có thể check xem e có miss case nào trong việc migration BS5 này ko với ạ
cc @phamgiang2510 @royleviindoo

@ngochuy97hp
Copy link

@ngochuy97hp a Huy có thể check xem e có miss case nào trong việc migration BS5 này ko với ạ cc @phamgiang2510 @royleviindoo

ok e

@phamgiang2510
Copy link

@duong77476 a chưa rõ có miss case nào hay ko nhưng bản staging của Viindoo (staging.viindoo.com) đang có rất nhiều template ngoài website mà chưa đc migrate sang BS5

@daiduongnguyen-odoo
Copy link
Author

@duong77476 a chưa rõ có miss case nào hay ko nhưng bản staging của Viindoo (staging.viindoo.com) đang có rất nhiều template ngoài website mà chưa đc migrate sang BS5

Có thể do đoạn query này chưa đủ ạ https://github.com/Viindoo/OpenUpgrade/blob/5601d50ab7378de73055a4658ea419666ebdf0cf/openupgrade_scripts/scripts/website/16.0.1.0/pre-migration.py#L82 , A check var xem e query như này cần thêm gì không a và có thể thử vs DB Viindoo xem nó được nhiêu bản ghi ạ
cc @royleviindoo

Comment on lines 151 to 152
_r("rounded-sm", "rounded-0"),
_r("rounded-lg", "rounded-1"),

Choose a reason for hiding this comment

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

Suggested change
_r("rounded-sm", "rounded-0"),
_r("rounded-lg", "rounded-1"),
_r("rounded-sm", "rounded-1"),
_r("rounded-lg", "rounded-3"),

_r("custom-control-input", "form-check-input"),
_r("custom-control-label", "form-check-label"),
_r("custom-switch", "form-switch"),
_r("custom-select", "form-select"),

Choose a reason for hiding this comment

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

viết kiểu này thì nó có đè cả custom-select-sm thành form-select-sm không? Cả -lg nữa, anh thấy trên staging chưa được.

Choose a reason for hiding this comment

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

@ngochuy97hp A cho e xin link cái page đó để có gì e test thử
cc @phamgiang2510 @royleviindoo

Choose a reason for hiding this comment

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

Choose a reason for hiding this comment

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

Nếu thế thì chắc phải viết hẳn ra kiểu _r("custom-select-sm", "form-select-sm") , tương tự với lg. Còn viết như trên nó chỉ selector đến đúng class đó xong replace thôi ạ

Choose a reason for hiding this comment

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

ok e

_r("mr", "me"),
# Forms
_r("custom-control", "form-control"),
_r("custom-check", "form-check"),

Choose a reason for hiding this comment

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

Suggested change
_r("custom-check", "form-check"),
_r("custom-checkbox", "form-check"),

Choose a reason for hiding this comment

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

custom-checkbox mới đúng.
Ở BS4 1 checkbox sẽ cần cặp custom-control custom-checkbox nhưng lên BS5 nó chỉ còn form-check
Ở trên có đoạn replace custom-control thành form-control. Như vậy sẽ thành form-control form-check -> lỗi

Choose a reason for hiding this comment

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

à đungs rồi, chắc e viết nhầm 👯

Comment on lines +104 to +123
_r("pl", "ps"),
_r("pr", "pe"),
_r("ml", "ms"),
_r("mr", "me"),

Choose a reason for hiding this comment

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

Mấy cái này nó cũng có sm với lg đấy

Choose a reason for hiding this comment

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

Đúng rồi , e vừa search có cả kiểu ms-1 xong cả ms-sm-2 nữa, theo như https://getbootstrap.com/docs/5.0/utilities/spacing/ đây sẽ có nhiều case hơn để cover ✌️

@daiduongnguyen-odoo daiduongnguyen-odoo force-pushed the master_imp_bs4_to_bs5_migration branch 2 times, most recently from a1759d0 to 4f04503 Compare August 25, 2023 07:53
@daiduongnguyen-odoo
Copy link
Author

@ngochuy97hp E đã sửa a nhé, a test được với db Viindoo thì có thể cài lại thư viện để test cc @phamgiang2510

Comment on lines 105 to 107
_r(
"text-justify", "text-center"
), # actually boostrap 5 only drop without any replacements

Choose a reason for hiding this comment

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

cái này chuyển thành style inline style="text-align: justify;"

Choose a reason for hiding this comment

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

Vâng a, e search thấy thằng odoo ko có class naò trong core cả, phải inline rồi :((

@daiduongnguyen-odoo daiduongnguyen-odoo force-pushed the master_imp_bs4_to_bs5_migration branch from 4f04503 to b0e1d53 Compare August 26, 2023 03:43
Comment on lines +107 to +112
_class_rp_by_inline(
selector="//*[contains(@class, 'text-justify')]",
selector_mode="xpath",
class_rp_by_inline={"text-justify": ["text-align: justify"]},
),
_r(class_rm="text-justify", class_add=""),

Choose a reason for hiding this comment

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

Comment on lines +207 to +215
if class_rp_by_inline:
inline_style = ""
for _, value in class_rp_by_inline.items():
for inline_css_style in value:
inline_style += inline_css_style + ";"
if "style" in node.attrib and node.attrib["style"]:
node.attrib["style"] += inline_style
else:
node.attrib["style"] = inline_style

Choose a reason for hiding this comment

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

Done #1 (comment)
Video test : https://drive.google.com/file/d/1zc2YCuE07w1hz3AFRbeVIpzAPlZ7xnVB/view?usp=sharing . E ko biết thêm class text-justify bằng website editor kiểu gì nên sửa view thủ công

huguesdk and others added 4 commits August 28, 2023 13:32
fix filtering of columns in delete_record_translations() for versions >=
16. the loop was wrong as it was changing the size of the list while
looping over it.
…umns_16

[FIX] fix columns filter in `delete_record_translations()` for versions >= 16
…irst_not_null-mle

[IMP] openupgrade_merge_records: add first_not_null operation on several field types
@daiduongnguyen-odoo daiduongnguyen-odoo force-pushed the master_imp_bs4_to_bs5_migration branch from b0e1d53 to b76b002 Compare August 30, 2023 01:16
Comment on lines +139 to +140
_r("custom-control", "form-control"),
_r("custom-checkbox", "form-check"),

Choose a reason for hiding this comment

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

@duong77476 trường hợp custom-checkbox thì cần chuyển cả custom-checkbox custom-control thành form-check. Để như này nó sẽ thành form-check form-control gây lỗi

Screenshot from 2023-09-07 16-07-00

Copy link
Author

Choose a reason for hiding this comment

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

ok anh để e xử ở con master của em

Copy link
Author

Choose a reason for hiding this comment

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

@ngochuy97hp Kiểu như này đúng ko a
_r( selector=".custom-checkbox + .custom-control", class_rm="custom-checkbox", class_add="form-check" ),
e chưa test , thấy thằng khác dùng như thế thôi
cc @royleviindoo

Copy link
Author

Choose a reason for hiding this comment

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

à có khi ko cần dấu +

Copy link
Author

Choose a reason for hiding this comment

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

Choose a reason for hiding this comment

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

anh chưa đọc cái hàm _r này miễn là 2 thằng này custom-checkbox custom-control biến thành form-check là được

Copy link
Author

Choose a reason for hiding this comment

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

@ngochuy97hp oh ok a, vậy nó nên là
_r( selector=".custom-checkbox .custom-control", class_rm="custom-checkbox custom-control", class_add="form-check", ),

nghĩa là selector đến element nào có 2 class trên, xóa 2 thk đó thay bằng thk form-check

Copy link
Author

Choose a reason for hiding this comment

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

Choose a reason for hiding this comment

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

uh đúng r e

Copy link
Author

Choose a reason for hiding this comment

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

Đã merged PR daiduongnguyen-odoo#7

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.

7 participants