mattermost-webapp
mattermost-webapp copied to clipboard
mattermost
MM-39217 : Add capability in plugin/apps forms to resize textarea via draggable corner
Summary
In this PR make the resizable props true from false in mentioned two files.
Ticket Link
Fixes https://github.com/mattermost/mattermost-server/issues/18621 Jira: https://mattermost.atlassian.net/browse/MM-39217
Related Pull Requests
- Has server changes (please link here)
- Has mobile changes (please link here)
Screenshots
Release Note
NONE
Hello @jatinmark,
Thanks for your pull request! A Core Committer will review your pull request soon. For code contributions, you can learn more about the review process here.
E2E tests not automatically triggered, because PR has no approval yet. Please ask a developer to review and then try again to attach the QA label.
@AshishDhama I am not.able to find the reasons for failed test . Test are failed because of the things i not even touch .But i am trying to understand the problem
@jatinmark hey i just quickly checked the failing test. Components are usually dependent on each other is some or the other way hence change in one place can also effect in other. 😃
Coming back to problem for failing tests.
That's because now the prop is resizable={true} in those files, test needs to be updated to show that. But i would like to rope in @mickmister to see if this is how this ticket needs to be done 🥂 . Because i think 🤔 we can avoid passing resizable to everything else except textarea. 0/5
So , thier is something needed to be done from my side or just i leave it till the review is not done .
@mickmister i have to make changes only in textarea to pass all the test ?
@mickmister i have to make changes only in textarea to pass all the test ?
There may be other issues with tests, but I'm asking for this change because the changes are currently affecting more elements than originally intended. It should only be applied to textareas for the purpose of user experience.
It looks like the tests may not ever use a textarea, so adding a test for that case in the files that are currently failing would be beneficial. What do you think about this @jatinmark?
@mickmister if adding test for textarea can remove the failing of test then its beneificial to add a test for it
@mickmister if adding test for textarea can remove the failing of test then its beneificial to add a test for it
The source code change we discussed (making it so only textareas are affected) will fix the failing tests. Adding a new test does not affect existing tests. Does this make sense?
@mickmister you mean to say that i have to only add test for textarea and then everything is fixed.
@mickmister you mean to say that i have to only add test for textarea and then everything is fixed.
@jatinmark There is another requirement that needs to be implemented in the source code that is currently edited in the PR, not in a test:
the
resizableprop should only passed when the field's subtype istextarea
@mickmister The current changes in PR is right ? And i have to make resizable prop pass in the textarea subtype and include a test for textarea.
@mickmister i included the tests in both the files and pass the resizable property please check it.
Successfully triggered e2e testing! https://git.internal.mattermost.com/qa/cypress-ui-automation/-/pipelines/253256
Mattermost test server created! :tada:
Access here: https://mattermost-webapp-pr-11321.test.mattermost.cloud
| Account Type | Username | Password |
|---|---|---|
| Admin | sysadmin | Sys@dmin123 |
| User | user-1 | User-1@123 |
Removing myself. After reading the description, @DHaussermann can you please take this PR (or Malik) as you are more familiar with plugin/apps forms - thanks 🙂
@M-ZubairAhmed i want to ask that their is something needed to be done from my side.
@jatinmark no actually lets wait for QA review to be done from @DHaussermann :)
Mattermost test server updated with git commit b8820311e776c30c36fc7f434a2cffcaf53e94c0.
Access here: https://mattermost-webapp-pr-11321.test.mattermost.cloud