Skip to content

Unify Control readOnly property case to camelCase#1810

Merged
mvorisek merged 8 commits intodevelopfrom
fix_readOnly
Aug 3, 2022
Merged

Unify Control readOnly property case to camelCase#1810
mvorisek merged 8 commits intodevelopfrom
fix_readOnly

Conversation

@mkrecek234
Copy link
Copy Markdown
Contributor

@mkrecek234 mkrecek234 commented Aug 2, 2022

change readonly to readOnly

@mkrecek234 mkrecek234 requested a review from mvorisek August 2, 2022 14:33
@mvorisek mvorisek removed their request for review August 2, 2022 14:39
@mvorisek
Copy link
Copy Markdown
Member

mvorisek commented Aug 2, 2022

@mkrecek234 what is your opinion on readonly? It seems it is one word keyword used in LC across many languages (PHP, C#, TS, ...).

@mkrecek234
Copy link
Copy Markdown
Contributor Author

mkrecek234 commented Aug 2, 2022

@mvorisek you can argue that "readonly" is common for many languages, but many languages don't camelCase. Thus, I clearly prefer the camelCase as it is according to the rules. In English, proper writing is "read-only", which also supports to camelCase it. Any other opinion @abbadon1334 @ibelar ? Not a major topic, but let's make ATK4 even more beautiful ;-)

'id' => $this->name . '_input',
'value' => $this->getValue(),
'readonly' => $this->readonly ? 'readonly' : false,
'readOnly' => $this->readOnly ? 'readonly' : false,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this is definitely wrong, readonly is canonical form of HTML attribute

@mvorisek mvorisek changed the title CamelCase readOnly control property Unify Control readOnly property case to camelCase Aug 3, 2022
@mvorisek
Copy link
Copy Markdown
Member

mvorisek commented Aug 3, 2022

@mvorisek you can argue that "readonly" is common for many languages, but many languages don't camelCase. Thus, I clearly prefer the camelCase as it is according to the rules. In English, proper writing is "read-only", which also supports to camelCase it. Any other opinion @abbadon1334 @ibelar ? Not a major topic, but let's make ATK4 even more beautiful ;-)

In atk4/* the camelCase naming is defined by the "most natural snake_case". Both "read_only" and "readonly" variants make sense (vs. "checkbox", almost noone would write "check_box").

I have done more reseach on "readonly" vs "readOnly" with Google. When I was renaming the property in atk4/data, I did not think much of it and simply converted the snake_case to camelCase. The case used in other projects in quite mixed, LC is used mainly when it is a "language attribute" which is almost always LC (case insensitive) only. When used as a property name, they are projects which use it like proposed. To unify the naming with atk4/data (atk4 ecosystem in general), I am ok with this change.

@mvorisek mvorisek merged commit e316b6c into develop Aug 3, 2022
@mvorisek mvorisek deleted the fix_readOnly branch August 3, 2022 03:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Development

Successfully merging this pull request may close these issues.

2 participants