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

Macro: #3495 - Is not possible to add RNA presets into the Favourite library #3584

Conversation

StarlaStarla
Copy link
Contributor

@StarlaStarla StarlaStarla commented Nov 17, 2023

How the feature works? / How did you fix the issue?

(Screenshots, videos, or GIFs, if applicable)
image
image

Check list

  • unit-tests written
  • e2e-tests written
  • documentation updated
  • PR name follows the pattern #1234 – issue name
  • branch name doesn't contain '#'
  • PR is linked with the issue
  • base branch (master or release/xx) is correct
  • task status changed to "Code review"
  • reviewers are notified about the pull request

@StarlaStarla StarlaStarla requested review from Nitvex and removed request for chgayane November 17, 2023 02:39
@StarlaStarla StarlaStarla force-pushed the 3495-macro-is-not-possible-to-add-rna-presets-into-the-favourite-library branch from b52115d to 8b5e7ad Compare November 17, 2023 07:59
@StarlaStarla StarlaStarla force-pushed the 3495-macro-is-not-possible-to-add-rna-presets-into-the-favourite-library branch from 8b5e7ad to 7844d35 Compare November 17, 2023 08:21
@StarlaStarla StarlaStarla requested review from MartaWilliams and removed request for Zhirnoff November 17, 2023 09:58
@MartaWilliams
Copy link
Collaborator

Tested:
2023-11-20_14h33_10
2023-11-20_14h33_18

});

test('Should have star when hover over RNA presets', async ({ page }) => {
await page.getByTestId('A_A_R_P').hover();
Copy link
Collaborator

Choose a reason for hiding this comment

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

My proposal is not to add a lot of screenshots. if tests can be written w/o it, let's do it.
So for this specific case – you can screenshot NOT whole page, but only specific DOM-element.

In addition, you can check the functionality, by checking DOM-elements. For example, you can open "favourite" tab and check that preset isn't there, then add it to "favourites" and check that it appears – no screenshot needed.

import { turnOnMacromoleculesEditor } from '@utils/macromolecules';
import { toggleRnaBuilderAccordion } from '@utils/macromolecules/rnaBuilder';

async function gotoRNA(page: Page) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I believe we already have this function somewhere. Can you move it to utils?

@@ -31,9 +33,15 @@ export const RNAContextMenu = () => {
switch (id) {
case 'duplicateandedit':
props.duplicatePreset(activePresetForContextMenu);
if (selectedTabIndex !== 2) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's use a constant/enum for this

setActivePreset,
setIsEditMode,
} from 'state/rna-builder';
import { scrollToSelectedPreset } from './RnaBuilder/RnaEditor/RnaEditor';
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's move styles to separate file?

dispatch(createNewPreset());
dispatch(setActiveRnaBuilderItem(RnaBuilderPresetsItem.Presets));
}
}, [activePreset]);

const expandEditor = () => {
setExpanded(!expanded);
if (!activePreset.presetInList) {
if (!activePreset?.presetInList) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you, please, also check the behaviour for editing presets?

  1. Just create any preset
  2. Click edit
  3. Try to change a name of preset
  4. Focus from input field is constantly lost after single character typing.

@StarlaStarla StarlaStarla force-pushed the 3495-macro-is-not-possible-to-add-rna-presets-into-the-favourite-library branch from 0baf296 to 711a640 Compare November 23, 2023 06:27
…ot-possible-to-add-rna-presets-into-the-favourite-library
…ot-possible-to-add-rna-presets-into-the-favourite-library
…ot-possible-to-add-rna-presets-into-the-favourite-library
…ot-possible-to-add-rna-presets-into-the-favourite-library
@StarlaStarla StarlaStarla force-pushed the 3495-macro-is-not-possible-to-add-rna-presets-into-the-favourite-library branch from 4612884 to 4c139de Compare November 30, 2023 08:46
@Nitvex Nitvex merged commit ceb5a86 into master Dec 1, 2023
@Nitvex Nitvex deleted the 3495-macro-is-not-possible-to-add-rna-presets-into-the-favourite-library branch December 1, 2023 09:54
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.

Macro: Is not possible to add RNA presets into the Favourite library
3 participants