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

Issue#932 #1007

Merged
merged 28 commits into from
Jan 19, 2024
Merged

Issue#932 #1007

merged 28 commits into from
Jan 19, 2024

Conversation

nagu165
Copy link
Contributor

@nagu165 nagu165 commented Jan 19, 2024

Problem :

When editing a file system, the form shows a disabled selector for the mount point field. Using a disabled widget does not make sense in this case because the mount point will never be editable. The disabled widget should be replaced by plain text.

Moreover, the selector currently uses a FormSelect component. It should be replaced by the Select component, similar to the file system type selector.

Solution :

  • Updated the MountPointFormSelector to use Select component Instead of the previous FormSelect component.

  • The select component adds more customizability and some advanced features as well.

  • Updated the mountpoint field to show the mountpath when currentVolume is defined and prompts the user to choose the mountpoint from the list of volume templates in case the currentVolume is undefined.

Testing :

Tested manually

Screenshots:

Before:
Screenshot 2024-01-09 232254
Screenshot 2024-01-09 232710

After:
image

image

nagu165 and others added 27 commits December 26, 2023 20:29
…Select component instead of the previous formSelect

- This change allows the developers to add more and better functionality and customizability to the Agama Linux Installer
- Replaced the disabled widget when editing a file system with text that describes which disc(volume) is mounted for the installation
…stead of the formSelect component

- Replaced the disabled widget at mountpoint field with text that tells the user which volume was selected and prompts the user to select a volume incase the currentVolume is undefined.
- This brings in better design and more flexibility to the Agama Linux Installer.
This reverts commit e2fc75e.
Fix eslint error that was previously missed.
Adapting them to the new "Mount point" widget behavior.
Avoiding the error below in the console output

>  console.error
>    Warning: React does not recognize the `isDisabled` prop on a DOM element.
To make it work as expected, dispatching an update event to keep the
form data in sync instead of having an internal state.
Not needed to check for the mount point control status anymore. Already
tested in the VolumeForm component.
…he user is allowed to select the mountPath

- Adds better user experience and improves consistency with code base.
- Improves the user experience and maintains consistency.
@nagu165
Copy link
Contributor Author

nagu165 commented Jan 19, 2024

I am really sorry about the previous pull request, I accidentally force-pushed it. Sorry for the trouble.

Thank you for your understanding.

@dgdavid
Copy link
Contributor

dgdavid commented Jan 19, 2024

Same as #973 which was closed by mistake.

Copy link
Contributor

@dgdavid dgdavid left a comment

Choose a reason for hiding this comment

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

@nagu165

In its current shape, the PR (former #973) LGTM 👍, thanks a ton for your interest in the project and, especially, for your contribution.

It has taken more time than both expected, though 😅 😅 However, beyond the contribution to the Agama project, hopefully it has been useful to your career. As advice for the future, don't forget to take as much time as necessary for understanding both, the tools you're using and the task you are trying to accomplish. No matter if you're contributing to a project or doing something for yourself. Taking care of the details is just as important as having a good grasp of the tool you are using at the time.

Too many words from my end already 😜

Let's merge it!

@dgdavid dgdavid merged commit ee1bae7 into agama-project:master Jan 19, 2024
1 check passed
@nagu165
Copy link
Contributor Author

nagu165 commented Jan 19, 2024

@nagu165

In its current shape, the PR (former #973) LGTM 👍, thanks a ton for your interest in the project and, especially, for your contribution.

It has taken more time than both expected, though 😅 😅 However, beyond the contribution to the Agama project, hopefully it has been useful to your career. As advice for the future, don't forget to take as much time as necessary for understanding both, the tools you're using and the task you are trying to accomplish. No matter if you're contributing to a project or doing something for yourself. Taking care of the details is just as important as having a good grasp of the tool you are using at the time.

Too many words from my end already 😜

Let's merge it!

Thank you so much for your guidance sir, As you have mentioned I learned a lot during this project and will strive to keep learning more.

@nagu165 nagu165 deleted the issue#932 branch January 20, 2024 06:14
@imobachgs imobachgs mentioned this pull request Feb 12, 2024
@imobachgs imobachgs mentioned this pull request May 17, 2024
imobachgs added a commit that referenced this pull request May 17, 2024
Prepare for releasing Agama 8. It includes the following pull requests:

* #884
* #886
* #914
* #918
* #956
* #957
* #958
* #959
* #960
* #961
* #962
* #963
* #964
* #965
* #966
* #969
* #970
* #976
* #977
* #978
* #979
* #980
* #981
* #983
* #984
* #985
* #986
* #988
* #991
* #992
* #995
* #996
* #997
* #999
* #1003
* #1004
* #1006
* #1007
* #1008
* #1009
* #1010
* #1011
* #1012
* #1014
* #1015
* #1016
* #1017
* #1020
* #1022
* #1023
* #1024
* #1025
* #1027
* #1028
* #1029
* #1030
* #1031
* #1032
* #1033
* #1034
* #1035
* #1036
* #1038
* #1039
* #1041
* #1042
* #1043
* #1045
* #1046
* #1047
* #1048
* #1052
* #1054
* #1056
* #1057
* #1060
* #1061
* #1062
* #1063
* #1064
* #1066
* #1067
* #1068
* #1069
* #1071
* #1072
* #1073
* #1074
* #1075
* #1079
* #1080
* #1081
* #1082
* #1085
* #1086
* #1087
* #1088
* #1089
* #1090
* #1091
* #1092
* #1093
* #1094
* #1095
* #1096
* #1097
* #1098
* #1099
* #1100
* #1102
* #1103
* #1104
* #1105
* #1106
* #1109
* #1110
* #1111
* #1112
* #1114
* #1116
* #1117
* #1118
* #1119
* #1120
* #1121
* #1122
* #1123
* #1125
* #1126
* #1127
* #1128
* #1129
* #1130
* #1131
* #1132
* #1133
* #1134
* #1135
* #1136
* #1138
* #1139
* #1140
* #1141
* #1142
* #1143
* #1144
* #1145
* #1146
* #1147
* #1148
* #1149
* #1151
* #1152
* #1153
* #1154
* #1155
* #1156
* #1157
* #1158
* #1160
* #1161
* #1162
* #1163
* #1164
* #1165
* #1166
* #1167
* #1168
* #1169
* #1170
* #1171
* #1172
* #1173
* #1174
* #1175
* #1177
* #1178
* #1180
* #1181
* #1182
* #1183
* #1184
* #1185
* #1187
* #1188
* #1189
* #1190
* #1191
* #1192
* #1193
* #1194
* #1195
* #1196
* #1198
* #1199
* #1200
* #1201
* #1203
* #1204
* #1205
* #1206
* #1207
* #1208
* #1209
* #1210
* #1211
* #1212
* #1213
* #1214
* #1215
* #1216
* #1217
* #1219
* #1220
* #1221
* #1222
* #1223
* #1224
* #1225
* #1226
* #1227
* #1229
@ancorgs
Copy link
Contributor

ancorgs commented May 17, 2024

JFYI, this pull request was mentioned in the announcement of Agama 8.

Thanks for your contribution!

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