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

[FIX] Config: check stylesheets scope #5836

Open
wants to merge 1 commit into
base: saas-18.2
Choose a base branch
from

Conversation

rrahir
Copy link
Collaborator

@rrahir rrahir commented Mar 6, 2025

The style that we define in our component can sometimes interfere within Odoo. This revision adds a step to force the style to be scoped to 'o-spreadsheet'.

Task: 4629441

Description:

description of this task, what is implemented and why it is implemented that way.

Task: TASK_ID

review checklist

  • feature is organized in plugin, or UI components
  • support of duplicate sheet (deep copy)
  • in model/core: ranges are Range object, and can be adapted (adaptRanges)
  • in model/UI: ranges are strings (to show the user)
  • undo-able commands (uses this.history.update)
  • multiuser-able commands (has inverse commands and transformations where needed)
  • new/updated/removed commands are documented
  • exportable in excel
  • translations (_t("qmsdf %s", abc))
  • unit tested
  • clean commented code
  • track breaking changes
  • doc is rebuild (npm run doc)
  • status is correct in Odoo

@robodoo
Copy link
Collaborator

robodoo commented Mar 6, 2025

Pull request status dashboard

@rrahir rrahir force-pushed the saas-18.2-enforce-spreadsheet-scope-scss-rar branch from 71c2087 to 22063a1 Compare March 6, 2025 09:13
then
exit 0; #THE USER WANTS TO CONTINUE
else
if [ $? -ne 0 ]; then
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

-ne <-> not equal

@rrahir rrahir force-pushed the saas-18.2-enforce-spreadsheet-scope-scss-rar branch from 22063a1 to e92ace9 Compare March 6, 2025 09:34
scssregexp='.*\.scss'
ignoreFileInstruction='\/\* husky-ignore \*\/'
filelist=()
for file in $(git diff --cached --name-only | grep -E $scssregexp); do
Copy link
Contributor

Choose a reason for hiding this comment

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

check is not enought :x This is not flagged as invalid css

image

Copy link
Collaborator Author

@rrahir rrahir Mar 7, 2025

Choose a reason for hiding this comment

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

I know, we discussed this limitation yesterday. it's more about having a generic check so the dev stays alert rather than having a full parser just for the fun.. or else I need to make a js script because standard grep is not smart enough to get it done but it seemed not necessary. @LucasLefevre
edit: punctuation re-edit: typos

Copy link
Contributor

Choose a reason for hiding this comment

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

Grep can probably still do the job in this case I think. We could maybe have a regex (startOfLine)(somethingThatIsn't.o-spreadshhet) Will only work is the CSS is correctly prettified but w/e.

This should work ?

^(\.(?!o-spreadsheet)|[a-zA-Z]+).*\{

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

you need to handle multi line grep which does not work on macos for instance, so no it does not work.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

i'll try to do a correct js script whenever I have the time

@rrahir rrahir force-pushed the saas-18.2-enforce-spreadsheet-scope-scss-rar branch 3 times, most recently from ecf51c2 to d707d0a Compare March 20, 2025 09:38
The style that we define in our component can sometimes interfere within
Odoo. This revision adds a step to force the style to be scoped to
'o-spreadsheet'.

Task: 4629441
@rrahir rrahir force-pushed the saas-18.2-enforce-spreadsheet-scope-scss-rar branch from d707d0a to 7352ddc Compare March 20, 2025 09:55
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